diff options
5 files changed, 216 insertions, 41 deletions
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 62150e9a1236..dad409316730 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 @@ -19,10 +19,13 @@ package com.android.systemui.statusbar.pipeline.mobile.domain.interactor import android.content.Context import android.telephony.CarrierConfigManager import android.telephony.SubscriptionManager +import android.telephony.SubscriptionManager.PROFILE_CLASS_PROVISIONING 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.flags.FeatureFlagsClassic +import com.android.systemui.flags.Flags.FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.log.table.logDiffsForTable import com.android.systemui.statusbar.pipeline.dagger.MobileSummaryLog @@ -121,6 +124,7 @@ constructor( userSetupRepo: UserSetupRepository, @Application private val scope: CoroutineScope, private val context: Context, + private val featureFlagsClassic: FeatureFlagsClassic, ) : MobileIconsInteractor { // Weak reference lookup for created interactors @@ -163,6 +167,20 @@ constructor( mobileConnectionsRepo.subscriptions /** + * Any filtering that we can do based purely on the info of each subscription. Currently this + * only applies the ProfileClass-based filter, but if we need other they can go here + */ + private val subscriptionsBasedFilteredSubs = + unfilteredSubscriptions.map { subs -> applyProvisioningFilter(subs) }.distinctUntilChanged() + + private fun applyProvisioningFilter(subs: List<SubscriptionModel>): List<SubscriptionModel> = + if (!featureFlagsClassic.isEnabled(FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS)) { + subs + } else { + subs.filter { it.profileClass != PROFILE_CLASS_PROVISIONING } + } + + /** * Generally, SystemUI wants to show iconography for each subscription that is listed by * [SubscriptionManager]. However, in the case of opportunistic subscriptions, we want to only * show a single representation of the pair of subscriptions. The docs define opportunistic as: @@ -177,48 +195,15 @@ constructor( */ override val filteredSubscriptions: Flow<List<SubscriptionModel>> = combine( - unfilteredSubscriptions, + subscriptionsBasedFilteredSubs, mobileConnectionsRepo.activeMobileDataSubscriptionId, connectivityRepository.vcnSubId, ) { unfilteredSubs, activeId, vcnSubId -> - // Based on the old logic, - if (unfilteredSubs.size != 2) { - return@combine unfilteredSubs - } - - val info1 = unfilteredSubs[0] - val info2 = unfilteredSubs[1] - - // Filtering only applies to subscriptions in the same group - if (info1.groupUuid == null || info1.groupUuid != info2.groupUuid) { - return@combine unfilteredSubs - } - - // If both subscriptions are primary, show both - if (!info1.isOpportunistic && !info2.isOpportunistic) { - return@combine unfilteredSubs - } - - // NOTE: at this point, we are now returning a single SubscriptionInfo - - // If carrier required, always show the icon of the primary subscription. - // Otherwise, show whichever subscription is currently active for internet. - if (carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault) { - // return the non-opportunistic info - return@combine if (info1.isOpportunistic) listOf(info2) else listOf(info1) - } else { - // It's possible for the subId of the VCN to disagree with the active subId in - // cases where the system has tried to switch but found no connection. In these - // scenarios, VCN will always have the subId that we want to use, so use that - // value instead of the activeId reported by telephony - val subIdToKeep = vcnSubId ?: activeId - - return@combine if (info1.subscriptionId == subIdToKeep) { - listOf(info1) - } else { - listOf(info2) - } - } + filterSubsBasedOnOpportunistic( + unfilteredSubs, + activeId, + vcnSubId, + ) } .distinctUntilChanged() .logDiffsForTable( @@ -229,6 +214,51 @@ constructor( ) .stateIn(scope, SharingStarted.WhileSubscribed(), listOf()) + private fun filterSubsBasedOnOpportunistic( + subList: List<SubscriptionModel>, + activeId: Int?, + vcnSubId: Int?, + ): List<SubscriptionModel> { + // Based on the old logic, + if (subList.size != 2) { + return subList + } + + val info1 = subList[0] + val info2 = subList[1] + + // Filtering only applies to subscriptions in the same group + if (info1.groupUuid == null || info1.groupUuid != info2.groupUuid) { + return subList + } + + // If both subscriptions are primary, show both + if (!info1.isOpportunistic && !info2.isOpportunistic) { + return subList + } + + // NOTE: at this point, we are now returning a single SubscriptionInfo + + // If carrier required, always show the icon of the primary subscription. + // Otherwise, show whichever subscription is currently active for internet. + if (carrierConfigTracker.alwaysShowPrimarySignalBarInOpportunisticNetworkDefault) { + // return the non-opportunistic info + return if (info1.isOpportunistic) listOf(info2) else listOf(info1) + } else { + // It's possible for the subId of the VCN to disagree with the active subId in + // cases where the system has tried to switch but found no connection. In these + // scenarios, VCN will always have the subId that we want to use, so use that + // value instead of the activeId reported by telephony + val subIdToKeep = vcnSubId ?: activeId + + return if (info1.subscriptionId == subIdToKeep) { + listOf(info1) + } else { + listOf(info2) + } + } + } + /** * 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). 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 12d57025ebb9..2060288c28a4 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 @@ -17,11 +17,16 @@ package com.android.systemui.statusbar.pipeline.mobile.domain.interactor import android.os.ParcelUuid +import android.telephony.SubscriptionManager import android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID +import android.telephony.SubscriptionManager.PROFILE_CLASS_PROVISIONING import android.telephony.SubscriptionManager.PROFILE_CLASS_UNSET import androidx.test.filters.SmallTest import com.android.settingslib.mobile.MobileMappings import com.android.systemui.SysuiTestCase +import com.android.systemui.coroutines.collectLastValue +import com.android.systemui.flags.FakeFeatureFlagsClassic +import com.android.systemui.flags.Flags import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel import com.android.systemui.statusbar.pipeline.mobile.data.repository.FakeMobileConnectionRepository @@ -58,6 +63,10 @@ class MobileIconsInteractorTest : SysuiTestCase() { private lateinit var connectionsRepository: FakeMobileConnectionsRepository private val userSetupRepository = FakeUserSetupRepository() private val mobileMappingsProxy = FakeMobileMappingsProxy() + private val flags = + FakeFeatureFlagsClassic().apply { + set(Flags.FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS, true) + } private val testDispatcher = UnconfinedTestDispatcher() private val testScope = TestScope(testDispatcher) @@ -100,6 +109,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { userSetupRepository, testScope.backgroundScope, context, + flags, ) } @@ -319,6 +329,123 @@ class MobileIconsInteractorTest : SysuiTestCase() { } @Test + fun filteredSubscriptions_doesNotFilterProvisioningWhenFlagIsFalse() = + testScope.runTest { + // GIVEN the flag is false + flags.set(Flags.FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS, false) + + // GIVEN 1 sub that is in PROFILE_CLASS_PROVISIONING + val sub1 = + SubscriptionModel( + subscriptionId = SUB_1_ID, + isOpportunistic = false, + carrierName = "Carrier 1", + profileClass = PROFILE_CLASS_PROVISIONING, + ) + + connectionsRepository.setSubscriptions(listOf(sub1)) + + // WHEN filtering is applied + val latest by collectLastValue(underTest.filteredSubscriptions) + + // THEN the provisioning sub is still present (unfiltered) + assertThat(latest).isEqualTo(listOf(sub1)) + } + + @Test + fun filteredSubscriptions_filtersOutProvisioningSubs() = + testScope.runTest { + val sub1 = + SubscriptionModel( + subscriptionId = SUB_1_ID, + isOpportunistic = false, + carrierName = "Carrier 1", + profileClass = PROFILE_CLASS_UNSET, + ) + val sub2 = + SubscriptionModel( + subscriptionId = SUB_2_ID, + isOpportunistic = false, + carrierName = "Carrier 2", + profileClass = SubscriptionManager.PROFILE_CLASS_PROVISIONING, + ) + + connectionsRepository.setSubscriptions(listOf(sub1, sub2)) + + val latest by collectLastValue(underTest.filteredSubscriptions) + + assertThat(latest).isEqualTo(listOf(sub1)) + } + + /** Note: I'm not sure if this will ever be the case, but we can test it at least */ + @Test + fun filteredSubscriptions_filtersOutProvisioningSubsBeforeOpportunistic() = + testScope.runTest { + // This is a contrived test case, where the active subId is the one that would + // also be filtered by opportunistic filtering. + + // GIVEN grouped, opportunistic subscriptions + val groupUuid = ParcelUuid(UUID.randomUUID()) + val sub1 = + SubscriptionModel( + subscriptionId = 1, + isOpportunistic = true, + groupUuid = groupUuid, + carrierName = "Carrier 1", + profileClass = PROFILE_CLASS_PROVISIONING, + ) + + val sub2 = + SubscriptionModel( + subscriptionId = 2, + isOpportunistic = true, + groupUuid = groupUuid, + carrierName = "Carrier 2", + profileClass = PROFILE_CLASS_UNSET, + ) + + // GIVEN active subId is 1 + connectionsRepository.setSubscriptions(listOf(sub1, sub2)) + connectionsRepository.setActiveMobileDataSubscriptionId(1) + + // THEN filtering of provisioning subs takes place first, and we result in sub2 + + val latest by collectLastValue(underTest.filteredSubscriptions) + + assertThat(latest).isEqualTo(listOf(sub2)) + } + + @Test + fun filteredSubscriptions_groupedPairAndNonProvisioned_groupedFilteringStillHappens() = + testScope.runTest { + // Grouped filtering only happens when the list of subs is length 2. In this case + // we'll show that filtering of provisioning subs happens before, and thus grouped + // filtering happens even though the unfiltered list is length 3 + val (sub1, sub3) = + createSubscriptionPair( + subscriptionIds = Pair(SUB_1_ID, SUB_3_ID), + opportunistic = Pair(true, true), + grouped = true, + ) + + val sub2 = + SubscriptionModel( + subscriptionId = 2, + isOpportunistic = true, + groupUuid = null, + carrierName = "Carrier 2", + profileClass = PROFILE_CLASS_PROVISIONING, + ) + + connectionsRepository.setSubscriptions(listOf(sub1, sub2, sub3)) + connectionsRepository.setActiveMobileDataSubscriptionId(1) + + val latest by collectLastValue(underTest.filteredSubscriptions) + + assertThat(latest).isEqualTo(listOf(sub1)) + } + + @Test fun activeDataConnection_turnedOn() = testScope.runTest { CONNECTION_1.setDataEnabled(true) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt index 1d5487f31e55..6a69d1fea993 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt @@ -70,7 +70,11 @@ class LocationBasedMobileIconViewModelTest : SysuiTestCase() { private lateinit var airplaneModeInteractor: AirplaneModeInteractor private val connectivityRepository = FakeConnectivityRepository() - private val flags = FakeFeatureFlagsClassic().also { it.set(Flags.NEW_NETWORK_SLICE_UI, false) } + private val flags = + FakeFeatureFlagsClassic().also { + it.set(Flags.NEW_NETWORK_SLICE_UI, false) + it.set(Flags.FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS, true) + } @Mock private lateinit var statusBarPipelineFlags: StatusBarPipelineFlags @Mock private lateinit var constants: ConnectivityConstants @@ -114,6 +118,7 @@ class LocationBasedMobileIconViewModelTest : SysuiTestCase() { FakeUserSetupRepository(), testScope.backgroundScope, context, + flags, ) interactor = 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 c831e62dd709..b39fc5b8fe21 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 @@ -82,7 +82,11 @@ class MobileIconViewModelTest : SysuiTestCase() { @Mock private lateinit var tableLogBuffer: TableLogBuffer @Mock private lateinit var carrierConfigTracker: CarrierConfigTracker - private val flags = FakeFeatureFlagsClassic().also { it.set(Flags.NEW_NETWORK_SLICE_UI, false) } + private val flags = + FakeFeatureFlagsClassic().also { + it.set(Flags.NEW_NETWORK_SLICE_UI, false) + it.set(Flags.FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS, true) + } private val testDispatcher = UnconfinedTestDispatcher() private val testScope = TestScope(testDispatcher) @@ -120,6 +124,7 @@ class MobileIconViewModelTest : SysuiTestCase() { FakeUserSetupRepository(), testScope.backgroundScope, context, + flags, ) interactor = diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt index c935dbb0ca1c..8405fb43e16a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ui/viewmodel/InternetTileViewModelTest.kt @@ -22,6 +22,8 @@ import com.android.systemui.SysuiTestCase import com.android.systemui.common.shared.model.ContentDescription.Companion.loadContentDescription import com.android.systemui.common.shared.model.Text import com.android.systemui.coroutines.collectLastValue +import com.android.systemui.flags.FakeFeatureFlagsClassic +import com.android.systemui.flags.Flags import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.qs.tileimpl.QSTileImpl.ResourceIcon import com.android.systemui.res.R @@ -75,6 +77,11 @@ class InternetTileViewModelTest : SysuiTestCase() { private val mobileConnectionRepository = FakeMobileConnectionRepository(SUB_1_ID, tableLogBuffer) + private val flags = + FakeFeatureFlagsClassic().also { + it.set(Flags.FILTER_PROVISIONING_NETWORK_SUBSCRIPTIONS, true) + } + private val internet = context.getString(R.string.quick_settings_internet_label) @Before @@ -101,6 +108,7 @@ class InternetTileViewModelTest : SysuiTestCase() { userSetupRepo, testScope.backgroundScope, context, + flags, ) underTest = |