summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Evan Laird <evanlaird@google.com> 2023-03-28 18:55:10 -0400
committer Evan Laird <evanlaird@google.com> 2023-04-05 14:06:41 -0400
commit1e79aacc1fe81b2136d964e541842f48ef0bd2f9 (patch)
tree1c86d9c0a047cfd4366d15ef52432b559a6c648d
parent21a25876b3104cfc70926cd90c69ef04b6d13dd8 (diff)
[Sb refactor] Remove the concept of `isDefault`
I feel like this CL deserves some introspection as to what is going on. INTRO The old pipeline has two main concepts that it tracks when it comes to knowing which subscription is special: `active` and `default`. They come from static methods on `SubscriptionManager`: getActiveDataSubscriptionId and getDefaultDataSubscriptionId. The old pipeline will map from `networkType` to icon group except for the following cases. For each subscription: 1. When !telephonyManager.isDataConnectionAllowed (!dataEnabled) AND the defaultDataSubId is not the current subscription, set the MobileIconGroup to NOT_DEFAULT_DATA. 2. When !dataEnabled AND this is the defaultDataSubId, set the MobileIconGroup to DATA_DISABLED. Note that neither of these icon groups represent icons, they have a resId of 0, meaning that later on when it comes time to use them, they will _not_ show any UI on screen, nor will their content descriptions be used. Also note that this is just system state stored in an UI view model-like class. THE PROBLEM So, the question becomes: what do we do with these two mobile icon groups in the new pipeline? Do we use the same input logic as the old pipeline and thread these two special-case MobileIconGroups through the MobileIconInteractor and then filter them out at the View Model? Or can we rework the system to be smarter? THIS CHANGE Note that from the intro we stated that both of these cases only apply when telephony reports `false` for `isDataConnectionAllowed`. Only _then_ does the old pipeline do anything with these icon groups. This means that as far as the UI is concerned, NOT_DEFAULT_DATA and DATA_DISABLED are equivalent (showing no UI). The only time that NOT_DEFAULT_DATA comes into play is when determining when to show the exclamation mark. Side note, there is a related bug in the old pipeline about checking NOT_DEFAULT_DATA. When NetworkControllerIpml is attempting to find out which controller is the data controller, it checks against the _active_ data subscriptio ID vs the _default_. This probably doesn't account for a huge amount of bugs, but it's worth mentioning this problem. With all of that out of the way, this change is rather simple: *remove the concept of `isDefault` from the pipeline*. The only thing we need to track is whether or not to show the data icon. The only remaining relevant piece to track was the exclamation mark on the triangle, but that field is covered elsewhere by a more appropriately-named field tracking the active data. Test: MobileIconInteractorTest Test: MobileIconViewModelTest Bug: 274724674 Bug: 270621876 Change-Id: I508a87e597cda5f9351053266701ebec84a0bcc3
-rw-r--r--packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractor.kt19
-rw-r--r--packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractor.kt6
-rw-r--r--packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt5
-rw-r--r--packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconsInteractor.kt1
-rw-r--r--packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconInteractorTest.kt23
-rw-r--r--packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt15
6 files changed, 14 insertions, 55 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 1c5ca6bbea21..b36ba3845fe9 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,7 +21,6 @@ import android.telephony.CarrierConfigManager
import com.android.settingslib.SignalIcon.MobileIconGroup
import com.android.settingslib.mobile.MobileIconCarrierIdOverrides
import com.android.settingslib.mobile.MobileIconCarrierIdOverridesImpl
-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.log.table.logDiffsForTable
@@ -41,7 +40,6 @@ import kotlinx.coroutines.flow.StateFlow
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.map
-import kotlinx.coroutines.flow.mapLatest
import kotlinx.coroutines.flow.stateIn
interface MobileIconInteractor {
@@ -125,7 +123,6 @@ class MobileIconInteractorImpl(
override val mobileIsDefault: StateFlow<Boolean>,
defaultMobileIconMapping: StateFlow<Map<String, MobileIconGroup>>,
defaultMobileIconGroup: StateFlow<MobileIconGroup>,
- defaultDataSubId: StateFlow<Int>,
override val isDefaultConnectionFailed: StateFlow<Boolean>,
override val isForceHidden: Flow<Boolean>,
connectionRepository: MobileConnectionRepository,
@@ -138,15 +135,6 @@ class MobileIconInteractorImpl(
override val isDataEnabled: StateFlow<Boolean> = connectionRepository.dataEnabled
- private val isDefault =
- defaultDataSubId
- .mapLatest { connectionRepository.subId == it }
- .stateIn(
- scope,
- SharingStarted.WhileSubscribed(),
- connectionRepository.subId == defaultDataSubId.value
- )
-
// True if there exists _any_ icon override for this carrierId. Note that overrides can include
// any or none of the icon groups defined in MobileMappings, so we still need to check on a
// per-network-type basis whether or not the given icon group is overridden
@@ -180,12 +168,7 @@ class MobileIconInteractorImpl(
connectionRepository.resolvedNetworkType,
defaultMobileIconMapping,
defaultMobileIconGroup,
- isDefault,
- ) { resolvedNetworkType, mapping, defaultGroup, isDefault ->
- if (!isDefault) {
- return@combine NOT_DEFAULT_DATA
- }
-
+ ) { resolvedNetworkType, mapping, defaultGroup ->
when (resolvedNetworkType) {
is ResolvedNetworkType.CarrierMergedNetworkType ->
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 7ec9ed8ccf2d..1e3122b1a515 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
@@ -76,9 +76,6 @@ interface MobileIconsInteractor {
/** True if the CDMA level should be preferred over the primary level. */
val alwaysUseCdmaLevel: StateFlow<Boolean>
- /** Tracks the subscriptionId set as the default for data connections */
- val defaultDataSubId: StateFlow<Int>
-
/** The icon mapping from network type to [MobileIconGroup] for the default subscription */
val defaultMobileIconMapping: StateFlow<Map<String, MobileIconGroup>>
@@ -186,8 +183,6 @@ constructor(
)
.stateIn(scope, SharingStarted.WhileSubscribed(), listOf())
- 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).
@@ -284,7 +279,6 @@ constructor(
mobileIsDefault,
defaultMobileIconMapping,
defaultMobileIconGroup,
- defaultDataSubId,
isDefaultConnectionFailed,
isForceHidden,
mobileConnectionsRepo.getRepoForSubId(subId),
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 50877eb4c72c..dce7bf21fadd 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
@@ -146,11 +146,10 @@ constructor(
combine(
iconInteractor.isDataConnected,
iconInteractor.isDataEnabled,
- iconInteractor.isDefaultConnectionFailed,
iconInteractor.alwaysShowDataRatIcon,
iconInteractor.mobileIsDefault,
- ) { dataConnected, dataEnabled, failedConnection, alwaysShow, mobileIsDefault ->
- alwaysShow || (dataConnected && dataEnabled && !failedConnection && mobileIsDefault)
+ ) { dataConnected, dataEnabled, alwaysShow, mobileIsDefault ->
+ alwaysShow || (dataEnabled && dataConnected && mobileIsDefault)
}
.distinctUntilChanged()
.logDiffsForTable(
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 d6fdad417b31..3ced7b2c4e6e 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
@@ -59,7 +59,6 @@ class FakeMobileIconsInteractor(
override val alwaysShowDataRatIcon = MutableStateFlow(false)
override val alwaysUseCdmaLevel = MutableStateFlow(false)
- override val defaultDataSubId = MutableStateFlow(DEFAULT_DATA_SUB_ID)
override val mobileIsDefault = MutableStateFlow(false)
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 8a515847a415..8d7f0f6035cc 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
@@ -245,27 +245,6 @@ class MobileIconInteractorTest : SysuiTestCase() {
}
@Test
- fun `icon group - checks default data`() =
- testScope.runTest {
- mobileIconsInteractor.defaultDataSubId.value = SUB_1_ID
- connectionRepository.resolvedNetworkType.value =
- DefaultNetworkType(mobileMappingsProxy.toIconKey(THREE_G))
-
- var latest: NetworkTypeIconModel? = null
- val job = underTest.networkTypeIconGroup.onEach { latest = it }.launchIn(this)
-
- assertThat(latest).isEqualTo(NetworkTypeIconModel.DefaultIcon(TelephonyIcons.THREE_G))
-
- // Default data sub id changes to something else
- mobileIconsInteractor.defaultDataSubId.value = 123
-
- assertThat(latest)
- .isEqualTo(NetworkTypeIconModel.DefaultIcon(TelephonyIcons.NOT_DEFAULT_DATA))
-
- job.cancel()
- }
-
- @Test
fun overrideIcon_usesCarrierIdOverride() =
testScope.runTest {
val overrides =
@@ -276,7 +255,6 @@ class MobileIconInteractorTest : SysuiTestCase() {
underTest = createInteractor(overrides)
- mobileIconsInteractor.defaultDataSubId.value = SUB_1_ID
connectionRepository.resolvedNetworkType.value =
DefaultNetworkType(mobileMappingsProxy.toIconKey(THREE_G))
@@ -506,7 +484,6 @@ class MobileIconInteractorTest : SysuiTestCase() {
mobileIconsInteractor.mobileIsDefault,
mobileIconsInteractor.defaultMobileIconMapping,
mobileIconsInteractor.defaultMobileIconGroup,
- mobileIconsInteractor.defaultDataSubId,
mobileIconsInteractor.isDefaultConnectionFailed,
mobileIconsInteractor.isForceHidden,
connectionRepository,
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 292e34c650bb..1b6ab4d7af96 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
@@ -268,10 +268,11 @@ class MobileIconViewModelTest : SysuiTestCase() {
}
@Test
- fun networkType_nullWhenDisabled() =
+ fun networkType_null_whenDisabled() =
testScope.runTest {
interactor.networkTypeIconGroup.value = NetworkTypeIconModel.DefaultIcon(THREE_G)
interactor.setIsDataEnabled(false)
+ interactor.mobileIsDefault.value = true
var latest: Icon? = null
val job = underTest.networkTypeIcon.onEach { latest = it }.launchIn(this)
@@ -281,15 +282,21 @@ class MobileIconViewModelTest : SysuiTestCase() {
}
@Test
- fun networkType_nullWhenFailedConnection() =
+ fun networkTypeIcon_notNull_whenEnabled() =
testScope.runTest {
+ val expected =
+ Icon.Resource(
+ THREE_G.dataType,
+ ContentDescription.Resource(THREE_G.dataContentDescription)
+ )
interactor.networkTypeIconGroup.value = NetworkTypeIconModel.DefaultIcon(THREE_G)
interactor.setIsDataEnabled(true)
- interactor.setIsFailedConnection(true)
+ interactor.isDataConnected.value = true
+ interactor.mobileIsDefault.value = true
var latest: Icon? = null
val job = underTest.networkTypeIcon.onEach { latest = it }.launchIn(this)
- assertThat(latest).isNull()
+ assertThat(latest).isEqualTo(expected)
job.cancel()
}