From e43cc99e308e6a19599c010a057336c4334120c1 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Fri, 20 Jan 2023 15:12:07 -0500 Subject: [PATCH 1/3] [Sb refactor] CarrierConfigRepository CarrierConfigRepository is meant to replace the old CarrierConfigTracker class and be easier to add new keys to. It exposes the ability to get a SystemUiCarrierConfig object which exposes tracked keys as individual Flows to be consumed in the modern-arch way. Test: CarrierConfigRepositoryTest Test: SystemUiCarrierConfigTest Test: MobileConnectionRepositoryTest Bug: 238425913 Change-Id: Id3ae744f62aae526bdd8573151632814617c1bd7 --- .../dagger/StatusBarPipelineModule.kt | 6 + .../data/model/SystemUiCarrierConfig.kt | 108 +++++++++++ .../repository/CarrierConfigCoreStartable.kt | 39 ++++ .../repository/CarrierConfigRepository.kt | 136 ++++++++++++++ .../prod/MobileConnectionRepositoryImpl.kt | 20 +- .../shared/ConnectivityPipelineLogger.kt | 9 + .../data/model/SystemUiCarrierConfigTest.kt | 120 ++++++++++++ .../repository/CarrierConfigRepositoryTest.kt | 172 ++++++++++++++++++ .../prod/MobileConnectionRepositoryTest.kt | 33 ++++ .../prod/MobileConnectionsRepositoryTest.kt | 12 ++ 10 files changed, 650 insertions(+), 5 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/SystemUiCarrierConfig.kt create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/CarrierConfigCoreStartable.kt create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/CarrierConfigRepository.kt create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/model/SystemUiCarrierConfigTest.kt create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/CarrierConfigRepositoryTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt index 0993ab3701f62..5f3b0dcb1654d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/dagger/StatusBarPipelineModule.kt @@ -25,6 +25,7 @@ import com.android.systemui.statusbar.pipeline.airplane.data.repository.Airplane import com.android.systemui.statusbar.pipeline.airplane.data.repository.AirplaneModeRepositoryImpl import com.android.systemui.statusbar.pipeline.airplane.ui.viewmodel.AirplaneModeViewModel import com.android.systemui.statusbar.pipeline.airplane.ui.viewmodel.AirplaneModeViewModelImpl +import com.android.systemui.statusbar.pipeline.mobile.data.repository.CarrierConfigCoreStartable import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionsRepository import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileRepositorySwitcher import com.android.systemui.statusbar.pipeline.mobile.data.repository.UserSetupRepository @@ -82,6 +83,11 @@ abstract class StatusBarPipelineModule { @ClassKey(MobileUiAdapter::class) abstract fun bindFeature(impl: MobileUiAdapter): CoreStartable + @Binds + @IntoMap + @ClassKey(CarrierConfigCoreStartable::class) + abstract fun bindCarrierConfigStartable(impl: CarrierConfigCoreStartable): CoreStartable + companion object { @Provides @SysUISingleton diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/SystemUiCarrierConfig.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/SystemUiCarrierConfig.kt new file mode 100644 index 0000000000000..8c82fbac90b80 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/SystemUiCarrierConfig.kt @@ -0,0 +1,108 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.statusbar.pipeline.mobile.data.model + +import android.os.PersistableBundle +import android.telephony.CarrierConfigManager.KEY_INFLATE_SIGNAL_STRENGTH_BOOL +import android.telephony.CarrierConfigManager.KEY_SHOW_OPERATOR_NAME_IN_STATUSBAR_BOOL +import androidx.annotation.VisibleForTesting +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow + +/** + * Represents, for a given subscription ID, the set of keys about which SystemUI cares. + * + * Upon first creation, this config represents only the default configuration (see + * [android.telephony.CarrierConfigManager.getDefaultConfig]). + * + * Upon request (see + * [com.android.systemui.statusbar.pipeline.mobile.data.repository.CarrierConfigRepository]), an + * instance of this class may be created for a given subscription Id, and will default to + * representing the default carrier configuration. However, once a carrier config is received for + * this [subId], all fields will reflect those in the received config, using [PersistableBundle]'s + * default of false for any config that is not present in the override. + * + * To keep things relatively simple, this class defines a wrapper around each config key which + * exposes a StateFlow for each config we care about. It also tracks whether or not it is + * using the default config for logging purposes. + * + * NOTE to add new keys to be tracked: + * 1. Define a new `private val` wrapping the key using [BooleanCarrierConfig] + * 2. Define a public `val` exposing the wrapped flow using [BooleanCarrierConfig.config] + * 3. Add the new [BooleanCarrierConfig] to the list of tracked configs, so they are properly + * updated when a new carrier config comes down + */ +class SystemUiCarrierConfig +internal constructor( + val subId: Int, + defaultConfig: PersistableBundle, +) { + @VisibleForTesting + var isUsingDefault = true + private set + + private val inflateSignalStrength = + BooleanCarrierConfig(KEY_INFLATE_SIGNAL_STRENGTH_BOOL, defaultConfig) + /** Flow tracking the [KEY_INFLATE_SIGNAL_STRENGTH_BOOL] carrier config */ + val shouldInflateSignalStrength: StateFlow = inflateSignalStrength.config + + private val showOperatorName = + BooleanCarrierConfig(KEY_SHOW_OPERATOR_NAME_IN_STATUSBAR_BOOL, defaultConfig) + /** Flow tracking the [KEY_SHOW_OPERATOR_NAME_IN_STATUSBAR_BOOL] config */ + val showOperatorNameInStatusBar: StateFlow = showOperatorName.config + + private val trackedConfigs = + listOf( + inflateSignalStrength, + showOperatorName, + ) + + /** Ingest a new carrier config, and switch all of the tracked keys over to the new values */ + fun processNewCarrierConfig(config: PersistableBundle) { + isUsingDefault = false + trackedConfigs.forEach { it.update(config) } + } + + /** For dumpsys, shortcut if we haven't overridden any keys */ + fun toStringConsideringDefaults(): String { + return if (isUsingDefault) { + "using defaults" + } else { + trackedConfigs.joinToString { it.toString() } + } + } + + override fun toString(): String = trackedConfigs.joinToString { it.toString() } +} + +/** Extracts [key] from the carrier config, and stores it in a flow */ +private class BooleanCarrierConfig( + val key: String, + defaultConfig: PersistableBundle, +) { + private val _configValue = MutableStateFlow(defaultConfig.getBoolean(key)) + val config = _configValue.asStateFlow() + + fun update(config: PersistableBundle) { + _configValue.value = config.getBoolean(key) + } + + override fun toString(): String { + return "$key=${config.value}" + } +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/CarrierConfigCoreStartable.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/CarrierConfigCoreStartable.kt new file mode 100644 index 0000000000000..af58999d9ddfe --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/CarrierConfigCoreStartable.kt @@ -0,0 +1,39 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.statusbar.pipeline.mobile.data.repository + +import com.android.systemui.CoreStartable +import com.android.systemui.dagger.qualifiers.Application +import javax.inject.Inject +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch + +/** + * Core startable which configures the [CarrierConfigRepository] to listen for updates for the + * lifetime of the process + */ +class CarrierConfigCoreStartable +@Inject +constructor( + private val carrierConfigRepository: CarrierConfigRepository, + @Application private val scope: CoroutineScope, +) : CoreStartable { + + override fun start() { + scope.launch { carrierConfigRepository.startObservingCarrierConfigUpdates() } + } +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/CarrierConfigRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/CarrierConfigRepository.kt new file mode 100644 index 0000000000000..5769f90ab6c7d --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/CarrierConfigRepository.kt @@ -0,0 +1,136 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.statusbar.pipeline.mobile.data.repository + +import android.content.IntentFilter +import android.os.PersistableBundle +import android.telephony.CarrierConfigManager +import android.telephony.SubscriptionManager +import android.util.SparseArray +import androidx.annotation.VisibleForTesting +import androidx.core.util.getOrElse +import androidx.core.util.isEmpty +import androidx.core.util.keyIterator +import com.android.systemui.Dumpable +import com.android.systemui.broadcast.BroadcastDispatcher +import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.dagger.qualifiers.Application +import com.android.systemui.dump.DumpManager +import com.android.systemui.statusbar.pipeline.mobile.data.model.SystemUiCarrierConfig +import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger +import java.io.PrintWriter +import javax.inject.Inject +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.SharedFlow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.mapNotNull +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.shareIn + +/** + * Meant to be the source of truth regarding CarrierConfigs. These are configuration objects defined + * on a per-subscriptionId basis, and do not trigger a device configuration event. + * + * Designed to supplant [com.android.systemui.util.CarrierConfigTracker]. + * + * See [SystemUiCarrierConfig] for details on how to add carrier config keys to be tracked + */ +@SysUISingleton +class CarrierConfigRepository +@Inject +constructor( + broadcastDispatcher: BroadcastDispatcher, + private val carrierConfigManager: CarrierConfigManager, + dumpManager: DumpManager, + logger: ConnectivityPipelineLogger, + @Application scope: CoroutineScope, +) : Dumpable { + private var isListening = false + private val defaultConfig: PersistableBundle by lazy { CarrierConfigManager.getDefaultConfig() } + // Used for logging the default config in the dumpsys + private val defaultConfigForLogs: SystemUiCarrierConfig by lazy { + SystemUiCarrierConfig(-1, defaultConfig) + } + + private val configs = SparseArray() + + init { + dumpManager.registerNormalDumpable(this) + } + + @VisibleForTesting + val carrierConfigStream: SharedFlow> = + broadcastDispatcher + .broadcastFlow(IntentFilter(CarrierConfigManager.ACTION_CARRIER_CONFIG_CHANGED)) { + intent, + _ -> + intent.getIntExtra( + CarrierConfigManager.EXTRA_SUBSCRIPTION_INDEX, + SubscriptionManager.INVALID_SUBSCRIPTION_ID + ) + } + .onEach { logger.logCarrierConfigChanged(it) } + .filter { SubscriptionManager.isValidSubscriptionId(it) } + .mapNotNull { subId -> + val config = carrierConfigManager.getConfigForSubId(subId) + config?.let { subId to it } + } + .shareIn(scope, SharingStarted.WhileSubscribed()) + + /** + * Start this repository observing broadcasts for **all** carrier configuration updates. Must be + * called in order to keep SystemUI in sync with [CarrierConfigManager]. + */ + suspend fun startObservingCarrierConfigUpdates() { + isListening = true + carrierConfigStream.collect { updateCarrierConfig(it.first, it.second) } + } + + /** Update or create the [SystemUiCarrierConfig] for subId with the override */ + private fun updateCarrierConfig(subId: Int, config: PersistableBundle) { + val configToUpdate = getOrCreateConfigForSubId(subId) + configToUpdate.processNewCarrierConfig(config) + } + + /** Gets a cached [SystemUiCarrierConfig], or creates a new one which will track the defaults */ + fun getOrCreateConfigForSubId(subId: Int): SystemUiCarrierConfig { + return configs.getOrElse(subId) { + val config = SystemUiCarrierConfig(subId, defaultConfig) + val carrierConfig = carrierConfigManager.getConfigForSubId(subId) + if (carrierConfig != null) config.processNewCarrierConfig(carrierConfig) + configs.put(subId, config) + config + } + } + + override fun dump(pw: PrintWriter, args: Array) { + pw.println("isListening: $isListening") + if (configs.isEmpty()) { + pw.println("no carrier configs loaded") + } else { + pw.println("Carrier configs by subId") + configs.keyIterator().forEach { + pw.println(" subId=$it") + pw.println(" config=${configs.get(it).toStringConsideringDefaults()}") + } + // Finally, print the default config + pw.println("Default config:") + pw.println(" $defaultConfigForLogs") + } + } +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt index 3f2ce4000ff1c..266430f13e33d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt @@ -44,8 +44,10 @@ import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameMode import com.android.systemui.statusbar.pipeline.mobile.data.model.ResolvedNetworkType.DefaultNetworkType import com.android.systemui.statusbar.pipeline.mobile.data.model.ResolvedNetworkType.OverrideNetworkType import com.android.systemui.statusbar.pipeline.mobile.data.model.ResolvedNetworkType.UnknownNetworkType +import com.android.systemui.statusbar.pipeline.mobile.data.model.SystemUiCarrierConfig import com.android.systemui.statusbar.pipeline.mobile.data.model.toDataConnectionType import com.android.systemui.statusbar.pipeline.mobile.data.model.toNetworkNameModel +import com.android.systemui.statusbar.pipeline.mobile.data.repository.CarrierConfigRepository import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository.Companion.DEFAULT_NUM_LEVELS import com.android.systemui.statusbar.pipeline.mobile.util.MobileMappingsProxy @@ -63,7 +65,7 @@ import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach @@ -82,6 +84,7 @@ class MobileConnectionRepositoryImpl( networkNameSeparator: String, private val telephonyManager: TelephonyManager, private val globalSettings: GlobalSettings, + systemUiCarrierConfig: SystemUiCarrierConfig, broadcastDispatcher: BroadcastDispatcher, globalMobileDataSettingChangedEvent: Flow, mobileMappingsProxy: MobileMappingsProxy, @@ -216,10 +219,15 @@ class MobileConnectionRepositoryImpl( .stateIn(scope, SharingStarted.WhileSubscribed(), state) } - // This will become variable based on [CarrierConfigManager.KEY_INFLATE_SIGNAL_STRENGTH_BOOL] - // once it's wired up inside of [CarrierConfigTracker]. - override val numberOfLevels: StateFlow = - flowOf(DEFAULT_NUM_LEVELS) + override val numberOfLevels = + systemUiCarrierConfig.shouldInflateSignalStrength + .map { shouldInflate -> + if (shouldInflate) { + DEFAULT_NUM_LEVELS + 1 + } else { + DEFAULT_NUM_LEVELS + } + } .stateIn(scope, SharingStarted.WhileSubscribed(), DEFAULT_NUM_LEVELS) /** Produces whenever the mobile data setting changes for this subId */ @@ -300,6 +308,7 @@ class MobileConnectionRepositoryImpl( private val telephonyManager: TelephonyManager, private val logger: ConnectivityPipelineLogger, private val globalSettings: GlobalSettings, + private val carrierConfigRepository: CarrierConfigRepository, private val mobileMappingsProxy: MobileMappingsProxy, @Background private val bgDispatcher: CoroutineDispatcher, @Application private val scope: CoroutineScope, @@ -318,6 +327,7 @@ class MobileConnectionRepositoryImpl( networkNameSeparator, telephonyManager.createForSubscriptionId(subId), globalSettings, + carrierConfigRepository.getOrCreateConfigForSubId(subId), broadcastDispatcher, globalMobileDataSettingChangedEvent, mobileMappingsProxy, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLogger.kt index 7c7ffaf5c6177..a25e52ba845fe 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLogger.kt @@ -231,6 +231,15 @@ constructor( ) } + fun logCarrierConfigChanged(subId: Int) { + buffer.log( + SB_LOGGING_TAG, + LogLevel.INFO, + { int1 = subId }, + { "onCarrierConfigChanged: subId=$int1" }, + ) + } + companion object { const val SB_LOGGING_TAG = "SbConnectivity" diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/model/SystemUiCarrierConfigTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/model/SystemUiCarrierConfigTest.kt new file mode 100644 index 0000000000000..63cb30ca4a33a --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/model/SystemUiCarrierConfigTest.kt @@ -0,0 +1,120 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.statusbar.pipeline.mobile.data.model + +import android.os.PersistableBundle +import android.telephony.CarrierConfigManager +import android.telephony.CarrierConfigManager.KEY_INFLATE_SIGNAL_STRENGTH_BOOL +import android.telephony.CarrierConfigManager.KEY_SHOW_OPERATOR_NAME_IN_STATUSBAR_BOOL +import androidx.test.filters.SmallTest +import com.android.systemui.SysuiTestCase +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.ExperimentalCoroutinesApi +import org.junit.Before +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +@SmallTest +class SystemUiCarrierConfigTest : SysuiTestCase() { + + lateinit var underTest: SystemUiCarrierConfig + + @Before + fun setUp() { + underTest = SystemUiCarrierConfig(SUB_1_ID, createTestConfig()) + } + + @Test + fun `process new config - reflected by isUsingDefault`() { + // Starts out using the defaults + assertThat(underTest.isUsingDefault).isTrue() + + // ANY new config means we're no longer tracking defaults + underTest.processNewCarrierConfig(createTestConfig()) + + assertThat(underTest.isUsingDefault).isFalse() + } + + @Test + fun `process new config - updates all flows`() { + assertThat(underTest.shouldInflateSignalStrength.value).isFalse() + assertThat(underTest.showOperatorNameInStatusBar.value).isFalse() + + underTest.processNewCarrierConfig( + configWithOverrides( + KEY_INFLATE_SIGNAL_STRENGTH_BOOL to true, + KEY_SHOW_OPERATOR_NAME_IN_STATUSBAR_BOOL to true, + ) + ) + + assertThat(underTest.shouldInflateSignalStrength.value).isTrue() + assertThat(underTest.showOperatorNameInStatusBar.value).isTrue() + } + + @Test + fun `process new config - defaults to false for config overrides`() { + // This case is only apparent when: + // 1. The default is true + // 2. The override config has no value for a given key + // In this case (per the old code) we would use the default value of false, despite there + // being no override key present in the override config + + underTest = + SystemUiCarrierConfig( + SUB_1_ID, + configWithOverrides( + KEY_INFLATE_SIGNAL_STRENGTH_BOOL to true, + KEY_SHOW_OPERATOR_NAME_IN_STATUSBAR_BOOL to true, + ) + ) + + assertThat(underTest.isUsingDefault).isTrue() + assertThat(underTest.shouldInflateSignalStrength.value).isTrue() + assertThat(underTest.showOperatorNameInStatusBar.value).isTrue() + + // Process a new config with no keys + underTest.processNewCarrierConfig(PersistableBundle()) + + assertThat(underTest.isUsingDefault).isFalse() + assertThat(underTest.shouldInflateSignalStrength.value).isFalse() + assertThat(underTest.showOperatorNameInStatusBar.value).isFalse() + } + + companion object { + private const val SUB_1_ID = 1 + + /** + * In order to keep us from having to update every place that might want to create a config, + * make sure to add new keys here + */ + fun createTestConfig() = + PersistableBundle().also { + it.putBoolean(CarrierConfigManager.KEY_INFLATE_SIGNAL_STRENGTH_BOOL, false) + it.putBoolean(CarrierConfigManager.KEY_SHOW_OPERATOR_NAME_IN_STATUSBAR_BOOL, false) + } + + /** Override the default config with the given (key, value) pair */ + fun configWithOverride(key: String, override: Boolean): PersistableBundle = + createTestConfig().also { it.putBoolean(key, override) } + + /** Override any number of configs from the default */ + fun configWithOverrides(vararg overrides: Pair) = + createTestConfig().also { config -> + overrides.forEach { (key, value) -> config.putBoolean(key, value) } + } + } +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/CarrierConfigRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/CarrierConfigRepositoryTest.kt new file mode 100644 index 0000000000000..521c67f20cfd0 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/CarrierConfigRepositoryTest.kt @@ -0,0 +1,172 @@ +/* + * Copyright (C) 2023 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.systemui.statusbar.pipeline.mobile.data.repository + +import android.content.Intent +import android.os.PersistableBundle +import android.telephony.CarrierConfigManager +import android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID +import androidx.test.filters.SmallTest +import com.android.dx.mockito.inline.extended.ExtendedMockito.mockitoSession +import com.android.systemui.SysuiTestCase +import com.android.systemui.dump.DumpManager +import com.android.systemui.statusbar.pipeline.mobile.data.model.SystemUiCarrierConfigTest.Companion.createTestConfig +import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger +import com.android.systemui.util.mockito.whenever +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import org.mockito.MockitoSession +import org.mockito.quality.Strictness + +@OptIn(ExperimentalCoroutinesApi::class) +@SmallTest +class CarrierConfigRepositoryTest : SysuiTestCase() { + private val testDispatcher = UnconfinedTestDispatcher() + private val testScope = TestScope(testDispatcher) + + private lateinit var underTest: CarrierConfigRepository + private lateinit var mockitoSession: MockitoSession + private lateinit var carrierConfigCoreStartable: CarrierConfigCoreStartable + + @Mock private lateinit var logger: ConnectivityPipelineLogger + @Mock private lateinit var carrierConfigManager: CarrierConfigManager + @Mock private lateinit var dumpManager: DumpManager + + @Before + fun setUp() { + MockitoAnnotations.initMocks(this) + mockitoSession = + mockitoSession() + .initMocks(this) + .mockStatic(CarrierConfigManager::class.java) + .strictness(Strictness.LENIENT) + .startMocking() + + whenever(CarrierConfigManager.getDefaultConfig()).thenReturn(DEFAULT_CONFIG) + + whenever(carrierConfigManager.getConfigForSubId(anyInt())).thenAnswer { invocation -> + when (invocation.getArgument(0) as Int) { + 1 -> CONFIG_1 + 2 -> CONFIG_2 + else -> null + } + } + + underTest = + CarrierConfigRepository( + fakeBroadcastDispatcher, + carrierConfigManager, + dumpManager, + logger, + testScope.backgroundScope, + ) + + carrierConfigCoreStartable = + CarrierConfigCoreStartable(underTest, testScope.backgroundScope) + } + + @After + fun tearDown() { + mockitoSession.finishMocking() + } + + @Test + fun `carrier config stream produces int-bundle pairs`() = + testScope.runTest { + var latest: Pair? = null + val job = underTest.carrierConfigStream.onEach { latest = it }.launchIn(this) + + sendConfig(SUB_ID_1) + assertThat(latest).isEqualTo(Pair(SUB_ID_1, CONFIG_1)) + + sendConfig(SUB_ID_2) + assertThat(latest).isEqualTo(Pair(SUB_ID_2, CONFIG_2)) + + job.cancel() + } + + @Test + fun `carrier config stream ignores invalid subscriptions`() = + testScope.runTest { + var latest: Pair? = null + val job = underTest.carrierConfigStream.onEach { latest = it }.launchIn(this) + + sendConfig(INVALID_SUBSCRIPTION_ID) + + assertThat(latest).isNull() + + job.cancel() + } + + @Test + fun `getOrCreateConfig - uses default config if no override`() { + val config = underTest.getOrCreateConfigForSubId(123) + assertThat(config.isUsingDefault).isTrue() + } + + @Test + fun `getOrCreateConfig - uses override if exists`() { + val config = underTest.getOrCreateConfigForSubId(SUB_ID_1) + assertThat(config.isUsingDefault).isFalse() + } + + @Test + fun `config - updates while config stream is collected`() = + testScope.runTest { + CONFIG_1.putBoolean(CarrierConfigManager.KEY_INFLATE_SIGNAL_STRENGTH_BOOL, false) + + carrierConfigCoreStartable.start() + + val config = underTest.getOrCreateConfigForSubId(SUB_ID_1) + assertThat(config.shouldInflateSignalStrength.value).isFalse() + + CONFIG_1.putBoolean(CarrierConfigManager.KEY_INFLATE_SIGNAL_STRENGTH_BOOL, true) + sendConfig(SUB_ID_1) + + assertThat(config.shouldInflateSignalStrength.value).isTrue() + } + + private fun sendConfig(subId: Int) { + fakeBroadcastDispatcher.registeredReceivers.forEach { receiver -> + receiver.onReceive( + context, + Intent(CarrierConfigManager.ACTION_CARRIER_CONFIG_CHANGED) + .putExtra(CarrierConfigManager.EXTRA_SUBSCRIPTION_INDEX, subId) + ) + } + } + + companion object { + private const val SUB_ID_1 = 1 + private const val SUB_ID_2 = 2 + + private val DEFAULT_CONFIG = createTestConfig() + private val CONFIG_1 = createTestConfig() + private val CONFIG_2 = createTestConfig() + } +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt index d6b8c0dbc59d8..b8dda780755e8 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt @@ -19,6 +19,7 @@ package com.android.systemui.statusbar.pipeline.mobile.data.repository.prod import android.content.Intent import android.os.UserHandle import android.provider.Settings +import android.telephony.CarrierConfigManager.KEY_INFLATE_SIGNAL_STRENGTH_BOOL import android.telephony.CellSignalStrengthCdma import android.telephony.NetworkRegistrationInfo import android.telephony.ServiceState @@ -59,6 +60,9 @@ import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameMode import com.android.systemui.statusbar.pipeline.mobile.data.model.ResolvedNetworkType.DefaultNetworkType import com.android.systemui.statusbar.pipeline.mobile.data.model.ResolvedNetworkType.OverrideNetworkType import com.android.systemui.statusbar.pipeline.mobile.data.model.ResolvedNetworkType.UnknownNetworkType +import com.android.systemui.statusbar.pipeline.mobile.data.model.SystemUiCarrierConfig +import com.android.systemui.statusbar.pipeline.mobile.data.model.SystemUiCarrierConfigTest.Companion.configWithOverride +import com.android.systemui.statusbar.pipeline.mobile.data.model.SystemUiCarrierConfigTest.Companion.createTestConfig import com.android.systemui.statusbar.pipeline.mobile.data.model.toNetworkNameModel import com.android.systemui.statusbar.pipeline.mobile.data.repository.FakeMobileConnectionsRepository import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository.Companion.DEFAULT_NUM_LEVELS @@ -100,6 +104,11 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { private val scope = CoroutineScope(IMMEDIATE) private val mobileMappings = FakeMobileMappingsProxy() private val globalSettings = FakeSettings() + private val systemUiCarrierConfig = + SystemUiCarrierConfig( + SUB_1_ID, + createTestConfig(), + ) @Before fun setUp() { @@ -117,6 +126,7 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { SEP, telephonyManager, globalSettings, + systemUiCarrierConfig, fakeBroadcastDispatcher, connectionsRepo.globalMobileDataSettingChangedEvent, mobileMappings, @@ -625,6 +635,29 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { job.cancel() } + @Test + fun `number of levels - uses carrier config`() = + runBlocking(IMMEDIATE) { + var latest: Int? = null + val job = underTest.numberOfLevels.onEach { latest = it }.launchIn(this) + + assertThat(latest).isEqualTo(DEFAULT_NUM_LEVELS) + + systemUiCarrierConfig.processNewCarrierConfig( + configWithOverride(KEY_INFLATE_SIGNAL_STRENGTH_BOOL, true) + ) + + assertThat(latest).isEqualTo(DEFAULT_NUM_LEVELS + 1) + + systemUiCarrierConfig.processNewCarrierConfig( + configWithOverride(KEY_INFLATE_SIGNAL_STRENGTH_BOOL, false) + ) + + assertThat(latest).isEqualTo(DEFAULT_NUM_LEVELS) + + job.cancel() + } + private fun getTelephonyCallbacks(): List { val callbackCaptor = argumentCaptor() Mockito.verify(telephonyManager).registerTelephonyCallback(any(), callbackCaptor.capture()) 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 db8172a5cacfa..f9567fc1eafc9 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 @@ -39,6 +39,7 @@ import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.log.table.TableLogBufferFactory 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.CarrierConfigRepository import com.android.systemui.statusbar.pipeline.mobile.data.repository.prod.FullMobileConnectionRepository.Factory.Companion.tableBufferLogName import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger @@ -80,6 +81,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { private lateinit var carrierMergedFactory: CarrierMergedConnectionRepository.Factory private lateinit var fullConnectionFactory: FullMobileConnectionRepository.Factory private lateinit var wifiRepository: FakeWifiRepository + private lateinit var carrierConfigRepository: CarrierConfigRepository @Mock private lateinit var connectivityManager: ConnectivityManager @Mock private lateinit var subscriptionManager: SubscriptionManager @Mock private lateinit var telephonyManager: TelephonyManager @@ -119,6 +121,15 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { wifiRepository = FakeWifiRepository() + carrierConfigRepository = + CarrierConfigRepository( + fakeBroadcastDispatcher, + mock(), + mock(), + logger, + scope, + ) + connectionFactory = MobileConnectionRepositoryImpl.Factory( fakeBroadcastDispatcher, @@ -129,6 +140,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { logger = logger, mobileMappingsProxy = mobileMappings, scope = scope, + carrierConfigRepository = carrierConfigRepository, ) carrierMergedFactory = CarrierMergedConnectionRepository.Factory( From fb60e153ffaba4ae1cf66e54eb62ab62b63d0ce3 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Fri, 20 Jan 2023 17:35:20 -0500 Subject: [PATCH 2/3] [Sb refactor] Implement DataEnabledChangedListener This CL changes the behavior of checking for mobile data enabled. The old pipeline would await telephony callbacks and then poll for `TelephonyManager.isDataConnectionAllowed`. Problem is that there was a race condition with watching the settings and polling this field, so it was often wrong on the first read. This change implements the newer TelephonyCallback.DataEnabledChangedListener, which can supplant the old polling method and simplify our repositories. Bug: 238425913 Bug: 260112365 Test: MobileConnectionRepositoryTest Change-Id: Ied9eea844501cd03100d85286993f85df671b37d --- data/etc/com.android.systemui.xml | 1 + packages/SystemUI/AndroidManifest.xml | 1 + .../repository/MobileConnectionsRepository.kt | 4 -- .../repository/MobileRepositorySwitcher.kt | 3 - .../demo/DemoMobileConnectionsRepository.kt | 2 - .../prod/FullMobileConnectionRepository.kt | 5 -- .../prod/MobileConnectionRepositoryImpl.kt | 58 ++++++------------- .../prod/MobileConnectionsRepositoryImpl.kt | 28 --------- .../shared/ConnectivityPipelineLogger.kt | 12 ++++ .../FakeMobileConnectionsRepository.kt | 7 --- .../MobileRepositorySwitcherTest.kt | 3 - .../FullMobileConnectionRepositoryTest.kt | 17 ------ .../prod/MobileConnectionRepositoryTest.kt | 45 ++------------ .../prod/MobileConnectionsRepositoryTest.kt | 24 -------- 14 files changed, 38 insertions(+), 172 deletions(-) diff --git a/data/etc/com.android.systemui.xml b/data/etc/com.android.systemui.xml index e0e13f59b7065..6dcee6d8bd311 100644 --- a/data/etc/com.android.systemui.xml +++ b/data/etc/com.android.systemui.xml @@ -49,6 +49,7 @@ + diff --git a/packages/SystemUI/AndroidManifest.xml b/packages/SystemUI/AndroidManifest.xml index e96aead597b3c..c1e0877d2fe5e 100644 --- a/packages/SystemUI/AndroidManifest.xml +++ b/packages/SystemUI/AndroidManifest.xml @@ -68,6 +68,7 @@ + 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 e0d156aa25f37..c640baa6e12e3 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 @@ -16,7 +16,6 @@ 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 @@ -53,9 +52,6 @@ interface MobileConnectionsRepository { /** Get or create a repository for the line of service for the given subscription ID */ fun getRepoForSubId(subId: Int): MobileConnectionRepository - /** Observe changes to the [Settings.Global.MOBILE_DATA] setting */ - val globalMobileDataSettingChangedEvent: Flow - /** * [Config] is an object that tracks relevant configuration flags for a given subscription ID. * In the case of [MobileMappings], it's hard-coded to check the default data subscription's 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 b93985604fb3e..7038a3bb0487f 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 @@ -156,9 +156,6 @@ constructor( realRepository.defaultMobileNetworkConnectivity.value ) - override val globalMobileDataSettingChangedEvent: Flow = - activeRepo.flatMapLatest { it.globalMobileDataSettingChangedEvent } - override fun getRepoForSubId(subId: Int): MobileConnectionRepository { if (isDemoMode.value) { return demoMobileConnectionsRepository.getRepoForSubId(subId) 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 108834521ebf0..58cd36e59d525 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 @@ -185,8 +185,6 @@ constructor( return CacheContainer(repo, lastMobileState = null) } - override val globalMobileDataSettingChangedEvent = MutableStateFlow(Unit) - fun startProcessingCommands() { mobileDemoCommandJob = scope.launch { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/FullMobileConnectionRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/FullMobileConnectionRepository.kt index 0f30ae249c31b..dd2cc928f7ad1 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/FullMobileConnectionRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/FullMobileConnectionRepository.kt @@ -26,7 +26,6 @@ import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConn import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow @@ -50,7 +49,6 @@ class FullMobileConnectionRepository( override val tableLogBuffer: TableLogBuffer, private val defaultNetworkName: NetworkNameModel, private val networkNameSeparator: String, - private val globalMobileDataSettingChangedEvent: Flow, @Application scope: CoroutineScope, private val mobileRepoFactory: MobileConnectionRepositoryImpl.Factory, private val carrierMergedRepoFactory: CarrierMergedConnectionRepository.Factory, @@ -84,7 +82,6 @@ class FullMobileConnectionRepository( tableLogBuffer, defaultNetworkName, networkNameSeparator, - globalMobileDataSettingChangedEvent, ) } @@ -150,7 +147,6 @@ class FullMobileConnectionRepository( startingIsCarrierMerged: Boolean, defaultNetworkName: NetworkNameModel, networkNameSeparator: String, - globalMobileDataSettingChangedEvent: Flow, ): FullMobileConnectionRepository { val mobileLogger = logFactory.getOrCreate(tableBufferLogName(subId), MOBILE_CONNECTION_BUFFER_SIZE) @@ -161,7 +157,6 @@ class FullMobileConnectionRepository( mobileLogger, defaultNetworkName, networkNameSeparator, - globalMobileDataSettingChangedEvent, scope, mobileRepoFactory, carrierMergedRepoFactory, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt index 266430f13e33d..1d0c9e9b8ed45 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt @@ -18,8 +18,6 @@ package com.android.systemui.statusbar.pipeline.mobile.data.repository.prod import android.content.Context import android.content.IntentFilter -import android.database.ContentObserver -import android.provider.Settings.Global import android.telephony.CellSignalStrength import android.telephony.CellSignalStrengthCdma import android.telephony.ServiceState @@ -53,7 +51,6 @@ import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConn import com.android.systemui.statusbar.pipeline.mobile.util.MobileMappingsProxy import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import com.android.systemui.statusbar.pipeline.shared.data.model.toMobileDataActivityModel -import com.android.systemui.util.settings.GlobalSettings import javax.inject.Inject import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope @@ -62,9 +59,11 @@ import kotlinx.coroutines.asExecutor import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.distinctUntilChanged +import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.merge @@ -83,10 +82,8 @@ class MobileConnectionRepositoryImpl( defaultNetworkName: NetworkNameModel, networkNameSeparator: String, private val telephonyManager: TelephonyManager, - private val globalSettings: GlobalSettings, systemUiCarrierConfig: SystemUiCarrierConfig, broadcastDispatcher: BroadcastDispatcher, - globalMobileDataSettingChangedEvent: Flow, mobileMappingsProxy: MobileMappingsProxy, bgDispatcher: CoroutineDispatcher, logger: ConnectivityPipelineLogger, @@ -117,7 +114,8 @@ class MobileConnectionRepositoryImpl( TelephonyCallback.DataConnectionStateListener, TelephonyCallback.DataActivityListener, TelephonyCallback.CarrierNetworkListener, - TelephonyCallback.DisplayInfoListener { + TelephonyCallback.DisplayInfoListener, + TelephonyCallback.DataEnabledListener { override fun onServiceStateChanged(serviceState: ServiceState) { logger.logOnServiceStateChanged(serviceState, subId) state = @@ -206,6 +204,12 @@ class MobileConnectionRepositoryImpl( state = state.copy(resolvedNetworkType = networkType) trySend(state) } + + override fun onDataEnabledChanged(enabled: Boolean, reason: Int) { + logger.logOnDataEnabledChanged(enabled, subId) + _dataEnabled.value = enabled + trySend(state) + } } telephonyManager.registerTelephonyCallback(bgDispatcher.asExecutor(), callback) awaitClose { telephonyManager.unregisterTelephonyCallback(callback) } @@ -230,24 +234,6 @@ class MobileConnectionRepositoryImpl( } .stateIn(scope, SharingStarted.WhileSubscribed(), DEFAULT_NUM_LEVELS) - /** Produces whenever the mobile data setting changes for this subId */ - private val localMobileDataSettingChangedEvent: Flow = conflatedCallbackFlow { - val observer = - object : ContentObserver(null) { - override fun onChange(selfChange: Boolean) { - trySend(Unit) - } - } - - globalSettings.registerContentObserver( - globalSettings.getUriFor("${Global.MOBILE_DATA}$subId"), - /* notifyForDescendants */ true, - observer - ) - - awaitClose { context.contentResolver.unregisterContentObserver(observer) } - } - /** * There are a few cases where we will need to poll [TelephonyManager] so we can update some * internal state where callbacks aren't provided. Any of those events should be merged into @@ -256,8 +242,6 @@ class MobileConnectionRepositoryImpl( private val telephonyPollingEvent: Flow = merge( telephonyCallbackEvent, - localMobileDataSettingChangedEvent, - globalMobileDataSettingChangedEvent, ) override val cdmaRoaming: StateFlow = @@ -284,21 +268,19 @@ class MobileConnectionRepositoryImpl( ) .stateIn(scope, SharingStarted.WhileSubscribed(), defaultNetworkName) - override val dataEnabled: StateFlow = run { - val initial = dataConnectionAllowed() - telephonyPollingEvent - .mapLatest { dataConnectionAllowed() } - .distinctUntilChanged() + private val _dataEnabled = MutableStateFlow(telephonyManager.isDataConnectionAllowed) + override val dataEnabled = + connectionInfo + // _dataEnabled is updated in connectionInfo's callback, so we need to make sure that + // this flow declares a dependency on that callback + .flatMapLatest { _dataEnabled } .logDiffsForTable( mobileLogger, columnPrefix = "", columnName = "dataEnabled", - initialValue = initial, + initialValue = _dataEnabled.value ) - .stateIn(scope, SharingStarted.WhileSubscribed(), initial) - } - - private fun dataConnectionAllowed(): Boolean = telephonyManager.isDataConnectionAllowed + .stateIn(scope, SharingStarted.WhileSubscribed(), _dataEnabled.value) class Factory @Inject @@ -307,7 +289,6 @@ class MobileConnectionRepositoryImpl( private val context: Context, private val telephonyManager: TelephonyManager, private val logger: ConnectivityPipelineLogger, - private val globalSettings: GlobalSettings, private val carrierConfigRepository: CarrierConfigRepository, private val mobileMappingsProxy: MobileMappingsProxy, @Background private val bgDispatcher: CoroutineDispatcher, @@ -318,7 +299,6 @@ class MobileConnectionRepositoryImpl( mobileLogger: TableLogBuffer, defaultNetworkName: NetworkNameModel, networkNameSeparator: String, - globalMobileDataSettingChangedEvent: Flow, ): MobileConnectionRepository { return MobileConnectionRepositoryImpl( context, @@ -326,10 +306,8 @@ class MobileConnectionRepositoryImpl( defaultNetworkName, networkNameSeparator, telephonyManager.createForSubscriptionId(subId), - globalSettings, carrierConfigRepository.getOrCreateConfigForSubId(subId), broadcastDispatcher, - globalMobileDataSettingChangedEvent, mobileMappingsProxy, bgDispatcher, logger, 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 39ad31f7eee82..10f48a36e7f64 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 @@ -19,14 +19,12 @@ package com.android.systemui.statusbar.pipeline.mobile.data.repository.prod import android.annotation.SuppressLint import android.content.Context import android.content.IntentFilter -import android.database.ContentObserver import android.net.ConnectivityManager import android.net.ConnectivityManager.NetworkCallback import android.net.Network import android.net.NetworkCapabilities import android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED import android.net.NetworkCapabilities.TRANSPORT_CELLULAR -import android.provider.Settings.Global.MOBILE_DATA import android.telephony.CarrierConfigManager import android.telephony.SubscriptionInfo import android.telephony.SubscriptionManager @@ -54,7 +52,6 @@ import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger 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.pairwise -import com.android.systemui.util.settings.GlobalSettings import javax.inject.Inject import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope @@ -87,7 +84,6 @@ constructor( private val logger: ConnectivityPipelineLogger, mobileMappingsProxy: MobileMappingsProxy, broadcastDispatcher: BroadcastDispatcher, - private val globalSettings: GlobalSettings, private val context: Context, @Background private val bgDispatcher: CoroutineDispatcher, @Application private val scope: CoroutineScope, @@ -222,29 +218,6 @@ constructor( ?: createRepositoryForSubId(subId).also { subIdRepositoryCache[subId] = it } } - /** - * In single-SIM devices, the [MOBILE_DATA] setting is phone-wide. For multi-SIM, the individual - * connection repositories also observe the URI for [MOBILE_DATA] + subId. - */ - override val globalMobileDataSettingChangedEvent: Flow = - conflatedCallbackFlow { - val observer = - object : ContentObserver(null) { - override fun onChange(selfChange: Boolean) { - trySend(Unit) - } - } - - globalSettings.registerContentObserver( - globalSettings.getUriFor(MOBILE_DATA), - true, - observer - ) - - awaitClose { context.contentResolver.unregisterContentObserver(observer) } - } - .logInputChange(logger, "globalMobileDataSettingChangedEvent") - @SuppressLint("MissingPermission") override val defaultMobileNetworkConnectivity: StateFlow = conflatedCallbackFlow { @@ -315,7 +288,6 @@ constructor( isCarrierMerged(subId), defaultNetworkName, networkNameSeparator, - globalMobileDataSettingChangedEvent, ) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLogger.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLogger.kt index a25e52ba845fe..6796a94cf6a92 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLogger.kt @@ -240,6 +240,18 @@ constructor( ) } + fun logOnDataEnabledChanged(enabled: Boolean, subId: Int) { + buffer.log( + SB_LOGGING_TAG, + LogLevel.INFO, + { + int1 = subId + bool1 = enabled + }, + { "onDataEnabledChanged: subId=$int1 enabled=$bool1" }, + ) + } + companion object { const val SB_LOGGING_TAG = "SbConnectivity" 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 0add905e2750c..cb9eb70d61dc1 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 @@ -71,9 +71,6 @@ class FakeMobileConnectionsRepository( ?: FakeMobileConnectionRepository(subId, tableLogBuffer).also { subIdRepos[subId] = it } } - private val _globalMobileDataSettingChangedEvent = MutableStateFlow(Unit) - override val globalMobileDataSettingChangedEvent = _globalMobileDataSettingChangedEvent - override val defaultDataSubRatConfig = MutableStateFlow(MobileMappings.Config()) private val _defaultMobileIconMapping = MutableStateFlow(TEST_MAPPING) @@ -94,10 +91,6 @@ class FakeMobileConnectionsRepository( _mobileConnectivity.value = model } - suspend fun triggerGlobalMobileDataSettingChangedEvent() { - _globalMobileDataSettingChangedEvent.emit(Unit) - } - fun setActiveMobileDataSubscriptionId(subId: Int) { _activeMobileDataSubscriptionId.value = subId } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt index 0859d140c3b4f..96cca44407694 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileRepositorySwitcherTest.kt @@ -40,7 +40,6 @@ import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.kotlinArgumentCaptor import com.android.systemui.util.mockito.mock import com.android.systemui.util.mockito.whenever -import com.android.systemui.util.settings.FakeSettings import com.android.systemui.util.time.FakeSystemClock import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.CoroutineScope @@ -85,7 +84,6 @@ class MobileRepositorySwitcherTest : SysuiTestCase() { @Mock private lateinit var demoModeController: DemoModeController @Mock private lateinit var dumpManager: DumpManager - private val globalSettings = FakeSettings() private val fakeNetworkEventsFlow = MutableStateFlow(null) private val mobileMappings = FakeMobileMappingsProxy() @@ -118,7 +116,6 @@ class MobileRepositorySwitcherTest : SysuiTestCase() { logger, mobileMappings, fakeBroadcastDispatcher, - globalSettings, context, IMMEDIATE, scope, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/FullMobileConnectionRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/FullMobileConnectionRepositoryTest.kt index c02a4dfd074cd..24b9f7d55da50 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/FullMobileConnectionRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/FullMobileConnectionRepositoryTest.kt @@ -23,9 +23,7 @@ import com.android.systemui.log.table.TableLogBufferFactory import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameModel import com.android.systemui.statusbar.pipeline.mobile.data.repository.FakeMobileConnectionRepository -import com.android.systemui.statusbar.pipeline.mobile.data.repository.FakeMobileConnectionsRepository import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository -import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.eq import com.android.systemui.util.mockito.mock @@ -33,7 +31,6 @@ import com.android.systemui.util.mockito.whenever import com.android.systemui.util.time.FakeSystemClock import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.test.TestScope @@ -57,22 +54,15 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { private val testDispatcher = UnconfinedTestDispatcher() private val testScope = TestScope(testDispatcher) - private val mobileMappings = FakeMobileMappingsProxy() private val tableLogBuffer = mock() private val mobileFactory = mock() private val carrierMergedFactory = mock() - private lateinit var connectionsRepo: FakeMobileConnectionsRepository - private val globalMobileDataSettingChangedEvent: Flow - get() = connectionsRepo.globalMobileDataSettingChangedEvent - private lateinit var mobileRepo: FakeMobileConnectionRepository private lateinit var carrierMergedRepo: FakeMobileConnectionRepository @Before fun setUp() { - connectionsRepo = FakeMobileConnectionsRepository(mobileMappings, tableLogBuffer) - mobileRepo = FakeMobileConnectionRepository(SUB_ID, tableLogBuffer) carrierMergedRepo = FakeMobileConnectionRepository(SUB_ID, tableLogBuffer) @@ -82,7 +72,6 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { any(), eq(DEFAULT_NAME), eq(SEP), - eq(globalMobileDataSettingChangedEvent), ) ) .thenReturn(mobileRepo) @@ -109,7 +98,6 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { tableLogBuffer, DEFAULT_NAME, SEP, - globalMobileDataSettingChangedEvent ) } @@ -310,7 +298,6 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { startingIsCarrierMerged = false, DEFAULT_NAME, SEP, - globalMobileDataSettingChangedEvent, ) val connection1Repeat = @@ -319,7 +306,6 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { startingIsCarrierMerged = false, DEFAULT_NAME, SEP, - globalMobileDataSettingChangedEvent, ) assertThat(connection1.tableLogBuffer) @@ -345,7 +331,6 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { startingIsCarrierMerged = false, DEFAULT_NAME, SEP, - globalMobileDataSettingChangedEvent, ) // WHEN a connection with the same sub ID but carrierMerged = true is created @@ -355,7 +340,6 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { startingIsCarrierMerged = true, DEFAULT_NAME, SEP, - globalMobileDataSettingChangedEvent, ) // THEN the same table is re-used @@ -374,7 +358,6 @@ class FullMobileConnectionRepositoryTest : SysuiTestCase() { tableLogBuffer, DEFAULT_NAME, SEP, - globalMobileDataSettingChangedEvent, testScope.backgroundScope, mobileFactory, carrierMergedFactory, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt index b8dda780755e8..77bdf9e307e85 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt @@ -17,8 +17,6 @@ package com.android.systemui.statusbar.pipeline.mobile.data.repository.prod import android.content.Intent -import android.os.UserHandle -import android.provider.Settings import android.telephony.CarrierConfigManager.KEY_INFLATE_SIGNAL_STRENGTH_BOOL import android.telephony.CellSignalStrengthCdma import android.telephony.NetworkRegistrationInfo @@ -74,7 +72,6 @@ import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.argumentCaptor import com.android.systemui.util.mockito.mock import com.android.systemui.util.mockito.whenever -import com.android.systemui.util.settings.FakeSettings import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -103,7 +100,6 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { private val scope = CoroutineScope(IMMEDIATE) private val mobileMappings = FakeMobileMappingsProxy() - private val globalSettings = FakeSettings() private val systemUiCarrierConfig = SystemUiCarrierConfig( SUB_1_ID, @@ -113,7 +109,6 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { @Before fun setUp() { MockitoAnnotations.initMocks(this) - globalSettings.userId = UserHandle.USER_ALL whenever(telephonyManager.subscriptionId).thenReturn(SUB_1_ID) connectionsRepo = FakeMobileConnectionsRepository(mobileMappings, tableLogger) @@ -125,10 +120,8 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { DEFAULT_NAME, SEP, telephonyManager, - globalSettings, systemUiCarrierConfig, fakeBroadcastDispatcher, - connectionsRepo.globalMobileDataSettingChangedEvent, mobileMappings, IMMEDIATE, logger, @@ -361,53 +354,27 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { @Test fun dataEnabled_initial_false() = - runBlocking(IMMEDIATE) { - whenever(telephonyManager.isDataConnectionAllowed).thenReturn(true) - - assertThat(underTest.dataEnabled.value).isFalse() - } - - @Test - fun dataEnabled_isEnabled_true() = - runBlocking(IMMEDIATE) { - whenever(telephonyManager.isDataConnectionAllowed).thenReturn(true) - val job = underTest.dataEnabled.launchIn(this) - - assertThat(underTest.dataEnabled.value).isTrue() - - job.cancel() - } - - @Test - fun dataEnabled_isDisabled() = runBlocking(IMMEDIATE) { whenever(telephonyManager.isDataConnectionAllowed).thenReturn(false) - val job = underTest.dataEnabled.launchIn(this) assertThat(underTest.dataEnabled.value).isFalse() - - job.cancel() } @Test - fun isDataConnectionAllowed_subIdSettingUpdate_valueUpdated() = + fun `is data enabled - tracks telephony callback`() = runBlocking(IMMEDIATE) { - val subIdSettingName = "${Settings.Global.MOBILE_DATA}$SUB_1_ID" - var latest: Boolean? = null val job = underTest.dataEnabled.onEach { latest = it }.launchIn(this) - // We don't read the setting directly, we query telephony when changes happen whenever(telephonyManager.isDataConnectionAllowed).thenReturn(false) - globalSettings.putInt(subIdSettingName, 0) - assertThat(latest).isFalse() + assertThat(underTest.dataEnabled.value).isFalse() - whenever(telephonyManager.isDataConnectionAllowed).thenReturn(true) - globalSettings.putInt(subIdSettingName, 1) + val callback = getTelephonyCallbackForType() + + callback.onDataEnabledChanged(true, 1) assertThat(latest).isTrue() - whenever(telephonyManager.isDataConnectionAllowed).thenReturn(false) - globalSettings.putInt(subIdSettingName, 0) + callback.onDataEnabledChanged(false, 1) assertThat(latest).isFalse() job.cancel() 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 f9567fc1eafc9..b73348c185603 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 @@ -23,7 +23,6 @@ import android.net.NetworkCapabilities import android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED import android.net.NetworkCapabilities.TRANSPORT_CELLULAR import android.os.ParcelUuid -import android.provider.Settings import android.telephony.CarrierConfigManager import android.telephony.SubscriptionInfo import android.telephony.SubscriptionManager @@ -50,7 +49,6 @@ import com.android.systemui.util.mockito.argumentCaptor import com.android.systemui.util.mockito.eq import com.android.systemui.util.mockito.mock import com.android.systemui.util.mockito.whenever -import com.android.systemui.util.settings.FakeSettings import com.google.common.truth.Truth.assertThat import java.util.UUID import kotlinx.coroutines.CoroutineScope @@ -91,7 +89,6 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { private val mobileMappings = FakeMobileMappingsProxy() private val scope = CoroutineScope(IMMEDIATE) - private val globalSettings = FakeSettings() @Before fun setUp() { @@ -136,7 +133,6 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { context = context, telephonyManager = telephonyManager, bgDispatcher = IMMEDIATE, - globalSettings = globalSettings, logger = logger, mobileMappingsProxy = mobileMappings, scope = scope, @@ -163,7 +159,6 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { logger, mobileMappings, fakeBroadcastDispatcher, - globalSettings, context, IMMEDIATE, scope, @@ -555,24 +550,6 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { job.cancel() } - @Test - fun globalMobileDataSettingsChangedEvent_producesOnSettingChange() = - runBlocking(IMMEDIATE) { - var produced = false - val job = - underTest.globalMobileDataSettingChangedEvent - .onEach { produced = true } - .launchIn(this) - - assertThat(produced).isFalse() - - globalSettings.putInt(Settings.Global.MOBILE_DATA, 0) - - assertThat(produced).isTrue() - - job.cancel() - } - @Test fun mobileConnectivity_isConnected_isNotValidated() = runBlocking(IMMEDIATE) { @@ -641,7 +618,6 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { logger, mobileMappings, fakeBroadcastDispatcher, - globalSettings, context, IMMEDIATE, scope, From 7eafef30da1e1eeae3d598c5e87878fe2a002d53 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Tue, 24 Jan 2023 11:09:28 -0500 Subject: [PATCH 3/3] [Sb refactor] Rework TelephonyCallback flows MobileConnectionRepository was doing a 1:1 mapping from its single TelephonyCallback listener to the `connectionInfo` model that it emitted from its flow. This wasn't great because there wasn't an easy way to add new callbacks which emitted from flows _other than_ the `connectionInfo` flow. In our case, we wanted `dataEnabled` to subscribe to the callback without starting collection on the `connectionInfo`. Secondly, we can now have a better polling event which is triggered on any telephony callback instead of the `onEach` approach which was used before (see `telephonyPollingEvent` for the change) Test: tests in com.android.systemui.statusbar.pipeline.mobile Bug: 238425913 Change-Id: I6843e949179f429eb8271e4021f884fbc079bf09 --- .../prod/MobileConnectionRepositoryImpl.kt | 206 +++++++++++------- .../prod/MobileConnectionRepositoryTest.kt | 3 - 2 files changed, 122 insertions(+), 87 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt index 1d0c9e9b8ed45..76fef358726d2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryImpl.kt @@ -59,15 +59,15 @@ import kotlinx.coroutines.asExecutor import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapLatest -import kotlinx.coroutines.flow.merge -import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.mapNotNull +import kotlinx.coroutines.flow.scan +import kotlinx.coroutines.flow.shareIn import kotlinx.coroutines.flow.stateIn /** @@ -84,7 +84,7 @@ class MobileConnectionRepositoryImpl( private val telephonyManager: TelephonyManager, systemUiCarrierConfig: SystemUiCarrierConfig, broadcastDispatcher: BroadcastDispatcher, - mobileMappingsProxy: MobileMappingsProxy, + private val mobileMappingsProxy: MobileMappingsProxy, bgDispatcher: CoroutineDispatcher, logger: ConnectivityPipelineLogger, mobileLogger: TableLogBuffer, @@ -103,8 +103,15 @@ class MobileConnectionRepositoryImpl( override val tableLogBuffer: TableLogBuffer = mobileLogger - override val connectionInfo: StateFlow = run { - var state = MobileConnectionModel() + /** + * This flow defines the single shared connection to system_server via TelephonyCallback. Any + * new callback should be added to this listener and funneled through callbackEvents via a data + * class. See [CallbackEvent] for defining new callbacks. + * + * The reason we need to do this is because TelephonyManager limits the number of registered + * listeners per-process, so we don't want to create a new listener for every callback. + */ + private val callbackEvents: SharedFlow = conflatedCallbackFlow { val callback = object : @@ -118,38 +125,12 @@ class MobileConnectionRepositoryImpl( TelephonyCallback.DataEnabledListener { override fun onServiceStateChanged(serviceState: ServiceState) { logger.logOnServiceStateChanged(serviceState, subId) - state = - state.copy( - isEmergencyOnly = serviceState.isEmergencyOnly, - isRoaming = serviceState.roaming, - operatorAlphaShort = serviceState.operatorAlphaShort, - isInService = Utils.isInService(serviceState), - ) - trySend(state) + trySend(CallbackEvent.OnServiceStateChanged(serviceState)) } override fun onSignalStrengthsChanged(signalStrength: SignalStrength) { logger.logOnSignalStrengthsChanged(signalStrength, subId) - val cdmaLevel = - signalStrength - .getCellSignalStrengths(CellSignalStrengthCdma::class.java) - .let { strengths -> - if (!strengths.isEmpty()) { - strengths[0].level - } else { - CellSignalStrength.SIGNAL_STRENGTH_NONE_OR_UNKNOWN - } - } - - val primaryLevel = signalStrength.level - - state = - state.copy( - cdmaLevel = cdmaLevel, - primaryLevel = primaryLevel, - isGsm = signalStrength.isGsm, - ) - trySend(state) + trySend(CallbackEvent.OnSignalStrengthChanged(signalStrength)) } override fun onDataConnectionStateChanged( @@ -157,70 +138,117 @@ class MobileConnectionRepositoryImpl( networkType: Int ) { logger.logOnDataConnectionStateChanged(dataState, networkType, subId) - state = - state.copy(dataConnectionState = dataState.toDataConnectionType()) - trySend(state) + trySend(CallbackEvent.OnDataConnectionStateChanged(dataState)) } override fun onDataActivity(direction: Int) { logger.logOnDataActivity(direction, subId) - state = - state.copy( - dataActivityDirection = direction.toMobileDataActivityModel() - ) - trySend(state) + trySend(CallbackEvent.OnDataActivity(direction)) } override fun onCarrierNetworkChange(active: Boolean) { logger.logOnCarrierNetworkChange(active, subId) - state = state.copy(carrierNetworkChangeActive = active) - trySend(state) + trySend(CallbackEvent.OnCarrierNetworkChange(active)) } override fun onDisplayInfoChanged( telephonyDisplayInfo: TelephonyDisplayInfo ) { logger.logOnDisplayInfoChanged(telephonyDisplayInfo, subId) - - val networkType = - if (telephonyDisplayInfo.networkType == NETWORK_TYPE_UNKNOWN) { - UnknownNetworkType - } else if ( - telephonyDisplayInfo.overrideNetworkType == - OVERRIDE_NETWORK_TYPE_NONE - ) { - DefaultNetworkType( - mobileMappingsProxy.toIconKey( - telephonyDisplayInfo.networkType - ) - ) - } else { - OverrideNetworkType( - mobileMappingsProxy.toIconKeyOverride( - telephonyDisplayInfo.overrideNetworkType - ) - ) - } - state = state.copy(resolvedNetworkType = networkType) - trySend(state) + trySend(CallbackEvent.OnDisplayInfoChanged(telephonyDisplayInfo)) } override fun onDataEnabledChanged(enabled: Boolean, reason: Int) { logger.logOnDataEnabledChanged(enabled, subId) - _dataEnabled.value = enabled - trySend(state) + trySend(CallbackEvent.OnDataEnabledChanged(enabled)) } } telephonyManager.registerTelephonyCallback(bgDispatcher.asExecutor(), callback) awaitClose { telephonyManager.unregisterTelephonyCallback(callback) } } - .onEach { telephonyCallbackEvent.tryEmit(Unit) } + .shareIn(scope, SharingStarted.WhileSubscribed()) + + private fun updateConnectionState( + prevState: MobileConnectionModel, + callbackEvent: CallbackEvent, + ): MobileConnectionModel = + when (callbackEvent) { + is CallbackEvent.OnServiceStateChanged -> { + val serviceState = callbackEvent.serviceState + prevState.copy( + isEmergencyOnly = serviceState.isEmergencyOnly, + isRoaming = serviceState.roaming, + operatorAlphaShort = serviceState.operatorAlphaShort, + isInService = Utils.isInService(serviceState), + ) + } + is CallbackEvent.OnSignalStrengthChanged -> { + val signalStrength = callbackEvent.signalStrength + val cdmaLevel = + signalStrength.getCellSignalStrengths(CellSignalStrengthCdma::class.java).let { + strengths -> + if (!strengths.isEmpty()) { + strengths[0].level + } else { + CellSignalStrength.SIGNAL_STRENGTH_NONE_OR_UNKNOWN + } + } + + val primaryLevel = signalStrength.level + + prevState.copy( + cdmaLevel = cdmaLevel, + primaryLevel = primaryLevel, + isGsm = signalStrength.isGsm, + ) + } + is CallbackEvent.OnDataConnectionStateChanged -> { + prevState.copy(dataConnectionState = callbackEvent.dataState.toDataConnectionType()) + } + is CallbackEvent.OnDataActivity -> { + prevState.copy( + dataActivityDirection = callbackEvent.direction.toMobileDataActivityModel() + ) + } + is CallbackEvent.OnCarrierNetworkChange -> { + prevState.copy(carrierNetworkChangeActive = callbackEvent.active) + } + is CallbackEvent.OnDisplayInfoChanged -> { + val telephonyDisplayInfo = callbackEvent.telephonyDisplayInfo + val networkType = + if (telephonyDisplayInfo.networkType == NETWORK_TYPE_UNKNOWN) { + UnknownNetworkType + } else if ( + telephonyDisplayInfo.overrideNetworkType == OVERRIDE_NETWORK_TYPE_NONE + ) { + DefaultNetworkType( + mobileMappingsProxy.toIconKey(telephonyDisplayInfo.networkType) + ) + } else { + OverrideNetworkType( + mobileMappingsProxy.toIconKeyOverride( + telephonyDisplayInfo.overrideNetworkType + ) + ) + } + prevState.copy(resolvedNetworkType = networkType) + } + is CallbackEvent.OnDataEnabledChanged -> { + // Not part of this object, handled in a separate flow + prevState + } + } + + override val connectionInfo = run { + val initial = MobileConnectionModel() + callbackEvents + .scan(initial, ::updateConnectionState) .logDiffsForTable( mobileLogger, columnPrefix = "MobileConnection ($subId)", - initialValue = state, + initialValue = initial, ) - .stateIn(scope, SharingStarted.WhileSubscribed(), state) + .stateIn(scope, SharingStarted.WhileSubscribed(), initial) } override val numberOfLevels = @@ -239,10 +267,7 @@ class MobileConnectionRepositoryImpl( * internal state where callbacks aren't provided. Any of those events should be merged into * this flow, which can be used to trigger the polling. */ - private val telephonyPollingEvent: Flow = - merge( - telephonyCallbackEvent, - ) + private val telephonyPollingEvent: Flow = callbackEvents.map { Unit } override val cdmaRoaming: StateFlow = telephonyPollingEvent @@ -268,19 +293,18 @@ class MobileConnectionRepositoryImpl( ) .stateIn(scope, SharingStarted.WhileSubscribed(), defaultNetworkName) - private val _dataEnabled = MutableStateFlow(telephonyManager.isDataConnectionAllowed) - override val dataEnabled = - connectionInfo - // _dataEnabled is updated in connectionInfo's callback, so we need to make sure that - // this flow declares a dependency on that callback - .flatMapLatest { _dataEnabled } + override val dataEnabled = run { + val initial = telephonyManager.isDataConnectionAllowed + callbackEvents + .mapNotNull { (it as? CallbackEvent.OnDataEnabledChanged)?.enabled } .logDiffsForTable( mobileLogger, columnPrefix = "", columnName = "dataEnabled", - initialValue = _dataEnabled.value + initialValue = initial ) - .stateIn(scope, SharingStarted.WhileSubscribed(), _dataEnabled.value) + .stateIn(scope, SharingStarted.WhileSubscribed(), initial) + } class Factory @Inject @@ -317,3 +341,17 @@ class MobileConnectionRepositoryImpl( } } } + +/** + * Wrap every [TelephonyCallback] we care about in a data class so we can accept them in a single + * shared flow and then split them back out into other flows. + */ +private sealed interface CallbackEvent { + data class OnServiceStateChanged(val serviceState: ServiceState) : CallbackEvent + data class OnSignalStrengthChanged(val signalStrength: SignalStrength) : CallbackEvent + data class OnDataConnectionStateChanged(val dataState: Int) : CallbackEvent + data class OnDataActivity(val direction: Int) : CallbackEvent + data class OnCarrierNetworkChange(val active: Boolean) : CallbackEvent + data class OnDisplayInfoChanged(val telephonyDisplayInfo: TelephonyDisplayInfo) : CallbackEvent + data class OnDataEnabledChanged(val enabled: Boolean) : CallbackEvent +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt index 77bdf9e307e85..3eb1aaa160332 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/prod/MobileConnectionRepositoryTest.kt @@ -395,8 +395,6 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { fun `roaming - cdma - queries telephony manager`() = runBlocking(IMMEDIATE) { var latest: Boolean? = null - // Start the telephony collection job so that cdmaRoaming starts updating - val telephonyJob = underTest.connectionInfo.launchIn(this) val job = underTest.cdmaRoaming.onEach { latest = it }.launchIn(this) val cb = getTelephonyCallbackForType() @@ -416,7 +414,6 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { assertThat(latest).isTrue() - telephonyJob.cancel() job.cancel() }