From 09ae54f5715b9f3d06c64e7319aa30a9007ba223 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Fri, 9 Dec 2022 10:45:48 -0500 Subject: [Sb refactor] Add location to mobile view models Test: tests in tests/src/com/android/systemui/pipeline/mobile Bug: 238425913 Change-Id: I8b06b0fba5b6a1467211a243311fc34091911f4a --- .../systemui/statusbar/phone/DemoStatusIcons.java | 5 +- .../statusbar/phone/StatusBarIconController.java | 13 ++- .../pipeline/mobile/ui/binder/MobileIconBinder.kt | 4 +- .../mobile/ui/view/ModernStatusBarMobileView.kt | 4 +- .../ui/viewmodel/LocationBasedMobileViewModel.kt | 63 +++++++++++++ .../mobile/ui/viewmodel/MobileIconViewModel.kt | 33 ++++--- .../mobile/ui/viewmodel/MobileIconsViewModel.kt | 19 ++-- .../LocationBasedMobileIconViewModelTest.kt | 105 +++++++++++++++++++++ .../mobile/ui/viewmodel/MobileIconViewModelTest.kt | 16 ++-- 9 files changed, 227 insertions(+), 35 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileViewModel.kt create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/DemoStatusIcons.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/DemoStatusIcons.java index c7be2193e9b5..5569718f4a4a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/DemoStatusIcons.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/DemoStatusIcons.java @@ -60,10 +60,12 @@ public class DemoStatusIcons extends StatusIconContainer implements DemoMode, Da private int mColor; private final MobileIconsViewModel mMobileIconsViewModel; + private final StatusBarLocation mLocation; public DemoStatusIcons( LinearLayout statusIcons, MobileIconsViewModel mobileIconsViewModel, + StatusBarLocation location, int iconSize ) { super(statusIcons.getContext()); @@ -71,6 +73,7 @@ public class DemoStatusIcons extends StatusIconContainer implements DemoMode, Da mIconSize = iconSize; mColor = DarkIconDispatcher.DEFAULT_ICON_TINT; mMobileIconsViewModel = mobileIconsViewModel; + mLocation = location; if (statusIcons instanceof StatusIconContainer) { setShouldRestrictIcons(((StatusIconContainer) statusIcons).isRestrictingIcons()); @@ -287,7 +290,7 @@ public class DemoStatusIcons extends StatusIconContainer implements DemoMode, Da ModernStatusBarMobileView view = ModernStatusBarMobileView.constructAndBind( mobileContext, "mobile", - mMobileIconsViewModel.viewModelForSub(subId) + mMobileIconsViewModel.viewModelForSub(subId, mLocation) ); // mobile always goes at the end diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java index df3ab493a4da..57cdc9eed577 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java @@ -359,6 +359,7 @@ public interface StatusBarIconController { // Whether or not these icons show up in dumpsys protected boolean mShouldLog = false; private StatusBarIconController mController; + private final StatusBarLocation mLocation; // Enables SystemUI demo mode to take effect in this group protected boolean mDemoable = true; @@ -381,6 +382,7 @@ public interface StatusBarIconController { mContext = group.getContext(); mIconSize = mContext.getResources().getDimensionPixelSize( com.android.internal.R.dimen.status_bar_icon_size); + mLocation = location; if (statusBarPipelineFlags.runNewMobileIconsBackend()) { // This starts the flow for the new pipeline, and will notify us of changes if @@ -394,7 +396,7 @@ public interface StatusBarIconController { if (statusBarPipelineFlags.runNewWifiIconBackend()) { // This starts the flow for the new pipeline, and will notify us of changes if // {@link StatusBarPipelineFlags#useNewWifiIcon} is also true. - mWifiViewModel = wifiUiAdapter.bindGroup(mGroup, location); + mWifiViewModel = wifiUiAdapter.bindGroup(mGroup, mLocation); } else { mWifiViewModel = null; } @@ -569,7 +571,7 @@ public interface StatusBarIconController { .constructAndBind( mobileContext, slot, - mMobileIconsViewModel.viewModelForSub(subId) + mMobileIconsViewModel.viewModelForSub(subId, mLocation) ); } @@ -705,7 +707,12 @@ public interface StatusBarIconController { } protected DemoStatusIcons createDemoStatusIcons() { - return new DemoStatusIcons((LinearLayout) mGroup, mMobileIconsViewModel, mIconSize); + return new DemoStatusIcons( + (LinearLayout) mGroup, + mMobileIconsViewModel, + mLocation, + mIconSize + ); } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt index 545e624273f1..ab442b5ab4de 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt @@ -30,7 +30,7 @@ import com.android.settingslib.graph.SignalDrawable import com.android.systemui.R import com.android.systemui.common.ui.binder.IconViewBinder import com.android.systemui.lifecycle.repeatWhenAttached -import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.MobileIconViewModel +import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.LocationBasedMobileViewModel import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.launch @@ -39,7 +39,7 @@ object MobileIconBinder { @JvmStatic fun bind( view: ViewGroup, - viewModel: MobileIconViewModel, + viewModel: LocationBasedMobileViewModel, ) { val activityContainer = view.requireViewById(R.id.inout_container) val activityIn = view.requireViewById(R.id.mobile_in) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/view/ModernStatusBarMobileView.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/view/ModernStatusBarMobileView.kt index 0ab7bcd96844..e86fee24fe4d 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/view/ModernStatusBarMobileView.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/view/ModernStatusBarMobileView.kt @@ -24,7 +24,7 @@ import com.android.systemui.R import com.android.systemui.statusbar.BaseStatusBarFrameLayout import com.android.systemui.statusbar.StatusBarIconView.STATE_ICON import com.android.systemui.statusbar.pipeline.mobile.ui.binder.MobileIconBinder -import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.MobileIconViewModel +import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.LocationBasedMobileViewModel import java.util.ArrayList class ModernStatusBarMobileView( @@ -71,7 +71,7 @@ class ModernStatusBarMobileView( fun constructAndBind( context: Context, slot: String, - viewModel: MobileIconViewModel, + viewModel: LocationBasedMobileViewModel, ): ModernStatusBarMobileView { return (LayoutInflater.from(context) .inflate(R.layout.status_bar_mobile_signal_group_new, null) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileViewModel.kt new file mode 100644 index 000000000000..070634dedd2f --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileViewModel.kt @@ -0,0 +1,63 @@ +/* + * Copyright (C) 2022 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.ui.viewmodel + +import android.graphics.Color +import com.android.systemui.statusbar.phone.StatusBarLocation +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flowOf + +/** + * A view model for an individual mobile icon that embeds the notion of a [StatusBarLocation]. This + * allows the mobile icon to change some view parameters at different locations + * + * @param commonImpl for convenience, this class wraps a base interface that can provides all of the + * common implementations between locations. See [MobileIconViewModel] + */ +abstract class LocationBasedMobileViewModel( + val commonImpl: MobileIconViewModelCommon, +) : MobileIconViewModelCommon by commonImpl { + abstract val tint: Flow + + companion object { + fun viewModelForLocation( + commonImpl: MobileIconViewModelCommon, + loc: StatusBarLocation, + ): LocationBasedMobileViewModel = + when (loc) { + StatusBarLocation.HOME -> HomeMobileIconViewModel(commonImpl) + StatusBarLocation.KEYGUARD -> KeyguardMobileIconViewModel(commonImpl) + StatusBarLocation.QS -> QsMobileIconViewModel(commonImpl) + } + } +} + +class HomeMobileIconViewModel( + commonImpl: MobileIconViewModelCommon, +) : MobileIconViewModelCommon, LocationBasedMobileViewModel(commonImpl) { + override val tint: Flow = flowOf(Color.CYAN) +} + +class QsMobileIconViewModel(commonImpl: MobileIconViewModelCommon) : + MobileIconViewModelCommon, LocationBasedMobileViewModel(commonImpl) { + override val tint: Flow = flowOf(Color.GREEN) +} + +class KeyguardMobileIconViewModel(commonImpl: MobileIconViewModelCommon) : + MobileIconViewModelCommon, LocationBasedMobileViewModel(commonImpl) { + override val tint: Flow = flowOf(Color.MAGENTA) +} 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 961283f57def..226409f66e15 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 @@ -34,6 +34,19 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapLatest +/** Common interface for all of the location-based mobile icon view models. */ +interface MobileIconViewModelCommon { + val subscriptionId: Int + /** An int consumable by [SignalDrawable] for display */ + val iconId: Flow + val roaming: Flow + /** The RAT icon (LTE, 3G, 5G, etc) to be displayed. Null if we shouldn't show anything */ + val networkTypeIcon: Flow + val activityInVisible: Flow + val activityOutVisible: Flow + val activityContainerVisible: Flow +} + /** * View model for the state of a single mobile icon. Each [MobileIconViewModel] will keep watch over * a single line of service via [MobileIconInteractor] and update the UI based on that @@ -41,24 +54,21 @@ import kotlinx.coroutines.flow.mapLatest * * There will be exactly one [MobileIconViewModel] per filtered subscription offered from * [MobileIconsInteractor.filteredSubscriptions] - * - * TODO: figure out where carrier merged and VCN models go (probably here?) */ @Suppress("EXPERIMENTAL_IS_NOT_ENABLED") @OptIn(ExperimentalCoroutinesApi::class) class MobileIconViewModel constructor( - val subscriptionId: Int, + override val subscriptionId: Int, iconInteractor: MobileIconInteractor, logger: ConnectivityPipelineLogger, constants: ConnectivityConstants, -) { +) : MobileIconViewModelCommon { /** Whether or not to show the error state of [SignalDrawable] */ private val showExclamationMark: Flow = iconInteractor.isDefaultDataEnabled.mapLatest { !it } - /** An int consumable by [SignalDrawable] for display */ - val iconId: Flow = + override val iconId: Flow = combine(iconInteractor.level, iconInteractor.numberOfLevels, showExclamationMark) { level, numberOfLevels, @@ -68,8 +78,7 @@ constructor( .distinctUntilChanged() .logOutputChange(logger, "iconId($subscriptionId)") - /** The RAT icon (LTE, 3G, 5G, etc) to be displayed. Null if we shouldn't show anything */ - val networkTypeIcon: Flow = + override val networkTypeIcon: Flow = combine( iconInteractor.networkTypeIconGroup, iconInteractor.isDataConnected, @@ -91,7 +100,7 @@ constructor( } } - val roaming: Flow = iconInteractor.isRoaming + override val roaming: Flow = iconInteractor.isRoaming private val activity: Flow = if (!constants.shouldShowActivityConfig) { @@ -100,9 +109,9 @@ constructor( iconInteractor.activity } - val activityInVisible: Flow = activity.map { it?.hasActivityIn ?: false } - val activityOutVisible: Flow = activity.map { it?.hasActivityOut ?: false } - val activityContainerVisible: Flow = + override val activityInVisible: Flow = activity.map { it?.hasActivityIn ?: false } + override val activityOutVisible: Flow = activity.map { it?.hasActivityOut ?: false } + override val activityContainerVisible: Flow = activity.map { it != null && (it.hasActivityIn || it.hasActivityOut) } val tint: Flow = flowOf(Color.CYAN) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModel.kt index 0b41d319f9dc..3beb96aa8cf4 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModel.kt @@ -18,6 +18,7 @@ package com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel +import com.android.systemui.statusbar.phone.StatusBarLocation import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.MobileIconsInteractor import com.android.systemui.statusbar.pipeline.mobile.ui.view.ModernStatusBarMobileView import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants @@ -40,13 +41,17 @@ constructor( private val constants: ConnectivityConstants, ) { /** TODO: do we need to cache these? */ - fun viewModelForSub(subId: Int): MobileIconViewModel = - MobileIconViewModel( - subId, - interactor.createMobileConnectionInteractorForSubId(subId), - logger, - constants, - ) + fun viewModelForSub(subId: Int, location: StatusBarLocation): LocationBasedMobileViewModel { + val common = + MobileIconViewModel( + subId, + interactor.createMobileConnectionInteractorForSubId(subId), + logger, + constants, + ) + + return LocationBasedMobileViewModel.viewModelForLocation(common, location) + } class Factory @Inject diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt new file mode 100644 index 000000000000..52232cbecf98 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt @@ -0,0 +1,105 @@ +/* + * Copyright (C) 2022 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.ui.viewmodel + +import androidx.test.filters.SmallTest +import com.android.settingslib.mobile.TelephonyIcons +import com.android.systemui.SysuiTestCase +import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.FakeMobileIconInteractor +import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.MobileIconViewModelTest.Companion.defaultSignal +import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants +import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger +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.Before +import org.junit.Test +import org.mockito.Mock +import org.mockito.MockitoAnnotations + +@Suppress("EXPERIMENTAL_IS_NOT_ENABLED") +@OptIn(ExperimentalCoroutinesApi::class) +@SmallTest +class LocationBasedMobileIconViewModelTest : SysuiTestCase() { + private lateinit var commonImpl: MobileIconViewModelCommon + private lateinit var homeIcon: HomeMobileIconViewModel + private lateinit var qsIcon: QsMobileIconViewModel + private lateinit var keyguardIcon: KeyguardMobileIconViewModel + private val interactor = FakeMobileIconInteractor() + @Mock private lateinit var logger: ConnectivityPipelineLogger + @Mock private lateinit var constants: ConnectivityConstants + + private val testDispatcher = UnconfinedTestDispatcher() + private val testScope = TestScope(testDispatcher) + + @Before + fun setUp() { + MockitoAnnotations.initMocks(this) + interactor.apply { + setLevel(1) + setIsDefaultDataEnabled(true) + setIsFailedConnection(false) + setIconGroup(TelephonyIcons.THREE_G) + setIsEmergencyOnly(false) + setNumberOfLevels(4) + isDataConnected.value = true + } + commonImpl = MobileIconViewModel(SUB_1_ID, interactor, logger, constants) + + homeIcon = HomeMobileIconViewModel(commonImpl) + qsIcon = QsMobileIconViewModel(commonImpl) + keyguardIcon = KeyguardMobileIconViewModel(commonImpl) + } + + @Test + fun `location based view models receive same icon id when common impl updates`() = + testScope.runTest { + var latestHome: Int? = null + val homeJob = homeIcon.iconId.onEach { latestHome = it }.launchIn(this) + + var latestQs: Int? = null + val qsJob = qsIcon.iconId.onEach { latestQs = it }.launchIn(this) + + var latestKeyguard: Int? = null + val keyguardJob = keyguardIcon.iconId.onEach { latestKeyguard = it }.launchIn(this) + + var expected = defaultSignal(level = 1) + + assertThat(latestHome).isEqualTo(expected) + assertThat(latestQs).isEqualTo(expected) + assertThat(latestKeyguard).isEqualTo(expected) + + interactor.setLevel(2) + expected = defaultSignal(level = 2) + + assertThat(latestHome).isEqualTo(expected) + assertThat(latestQs).isEqualTo(expected) + assertThat(latestKeyguard).isEqualTo(expected) + + homeJob.cancel() + qsJob.cancel() + keyguardJob.cancel() + } + + companion object { + private const val SUB_1_ID = 1 + } +} 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 415ce75345b2..76437379e1a7 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 @@ -340,16 +340,16 @@ class MobileIconViewModelTest : SysuiTestCase() { containerJob.cancel() } - /** Convenience constructor for these tests */ - private fun defaultSignal( - level: Int = 1, - connected: Boolean = true, - ): Int { - return SignalDrawable.getState(level, /* numLevels */ 4, !connected) - } - companion object { private val IMMEDIATE = Dispatchers.Main.immediate private const val SUB_1_ID = 1 + + /** Convenience constructor for these tests */ + fun defaultSignal( + level: Int = 1, + connected: Boolean = true, + ): Int { + return SignalDrawable.getState(level, /* numLevels */ 4, !connected) + } } } -- cgit v1.2.3-59-g8ed1b From bcfa7ada7cb402ba967cd22a9a2e4f5e7a7bd570 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Mon, 12 Dec 2022 17:59:06 -0500 Subject: [Sb refactor] Move shouldShowActivityConfig to ConnectivityConstants This field was needed both for mobile and wifi. This CL updates the wifi pipeline to use the `ConnectivityConstants` version to keep things centralized. Test: WifiViewModelTest Bug: 238425913 Change-Id: Ia681c648a10c1daba48916ca48a41e0aec236a79 --- .../pipeline/wifi/shared/WifiConstants.kt | 4 ---- .../pipeline/wifi/ui/viewmodel/WifiViewModel.kt | 2 +- .../wifi/ui/viewmodel/WifiViewModelTest.kt | 24 +++++++++++----------- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/WifiConstants.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/WifiConstants.kt index 3c0eb910ad89..4f7fe28c1e7c 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/WifiConstants.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/WifiConstants.kt @@ -38,16 +38,12 @@ class WifiConstants @Inject constructor( dumpManager.registerDumpable("${SB_LOGGING_TAG}WifiConstants", this) } - /** True if we should show the activityIn/activityOut icons and false otherwise. */ - val shouldShowActivityConfig = context.resources.getBoolean(R.bool.config_showActivity) - /** True if we should always show the wifi icon when wifi is enabled and false otherwise. */ val alwaysShowIconIfEnabled = context.resources.getBoolean(R.bool.config_showWifiIndicatorWhenEnabled) override fun dump(pw: PrintWriter, args: Array) { pw.apply { - println("shouldShowActivityConfig=$shouldShowActivityConfig") println("alwaysShowIconIfEnabled=$alwaysShowIconIfEnabled") } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModel.kt index 07a7595a2e00..ab464cc78905 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModel.kt @@ -148,7 +148,7 @@ constructor( /** The wifi activity status. Null if we shouldn't display the activity status. */ private val activity: Flow = - if (!wifiConstants.shouldShowActivityConfig) { + if (!connectivityConstants.shouldShowActivityConfig) { flowOf(null) } else { combine(interactor.activity, interactor.ssid) { activity, ssid -> diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelTest.kt index b47f177bbf24..41584347c0f2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelTest.kt @@ -146,7 +146,7 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun activity_showActivityConfigFalse_outputsFalse() = runBlocking(IMMEDIATE) { - whenever(wifiConstants.shouldShowActivityConfig).thenReturn(false) + whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(false) createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) @@ -183,7 +183,7 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun activity_showActivityConfigFalse_noUpdatesReceived() = runBlocking(IMMEDIATE) { - whenever(wifiConstants.shouldShowActivityConfig).thenReturn(false) + whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(false) createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) @@ -225,7 +225,7 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun activity_nullSsid_outputsFalse() = runBlocking(IMMEDIATE) { - whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true) + whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(true) createAndSetViewModel() wifiRepository.setWifiNetwork(WifiNetworkModel.Active(NETWORK_ID, ssid = null)) @@ -268,7 +268,7 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun activity_allLocationViewModelsReceiveSameData() = runBlocking(IMMEDIATE) { - whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true) + whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(true) createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) @@ -308,7 +308,7 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun activityIn_hasActivityInTrue_outputsTrue() = runBlocking(IMMEDIATE) { - whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true) + whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(true) createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) @@ -330,7 +330,7 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun activityIn_hasActivityInFalse_outputsFalse() = runBlocking(IMMEDIATE) { - whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true) + whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(true) createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) @@ -352,7 +352,7 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun activityOut_hasActivityOutTrue_outputsTrue() = runBlocking(IMMEDIATE) { - whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true) + whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(true) createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) @@ -374,7 +374,7 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun activityOut_hasActivityOutFalse_outputsFalse() = runBlocking(IMMEDIATE) { - whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true) + whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(true) createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) @@ -396,7 +396,7 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun activityContainer_hasActivityInTrue_outputsTrue() = runBlocking(IMMEDIATE) { - whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true) + whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(true) createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) @@ -418,7 +418,7 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun activityContainer_hasActivityOutTrue_outputsTrue() = runBlocking(IMMEDIATE) { - whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true) + whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(true) createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) @@ -440,7 +440,7 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun activityContainer_inAndOutTrue_outputsTrue() = runBlocking(IMMEDIATE) { - whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true) + whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(true) createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) @@ -462,7 +462,7 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun activityContainer_inAndOutFalse_outputsFalse() = runBlocking(IMMEDIATE) { - whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true) + whenever(connectivityConstants.shouldShowActivityConfig).thenReturn(true) createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) -- cgit v1.2.3-59-g8ed1b From 4e53da6d8d0480de15dc3f05e237f0e5efc39df1 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Thu, 15 Dec 2022 17:56:32 -0500 Subject: [Sb refactor] Add logging to mobile connections pipeline Add logging to all inputs into the mobile connections pipeline, including the TelephonyCallback that is used to calculate most of the relevant state for an individual icon Test: MobileConnectionsRepositoryTest Bug: 238425913 Bug: 240569788 Change-Id: I6eccf2d21f942a19dde8543a7926e3ce1c069db4 --- .../mobile/data/model/MobileConnectionModel.kt | 72 ++++++++- .../pipeline/mobile/data/model/NetworkNameModel.kt | 32 +++- .../prod/MobileConnectionRepositoryImpl.kt | 59 +++++++- .../prod/MobileConnectionsRepositoryImpl.kt | 55 ++++--- .../pipeline/shared/ConnectivityPipelineLogger.kt | 166 ++++++++++++++------- .../mobile/data/model/MobileConnectionModelTest.kt | 93 ++++++++++++ .../prod/MobileConnectionRepositoryTest.kt | 3 + .../prod/MobileConnectionsRepositoryTest.kt | 36 +++++ 8 files changed, 431 insertions(+), 85 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/model/MobileConnectionModelTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/MobileConnectionModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/MobileConnectionModel.kt index 6c37f94007cb..1aa954ff48cf 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/MobileConnectionModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/MobileConnectionModel.kt @@ -26,6 +26,8 @@ import android.telephony.TelephonyCallback.ServiceStateListener import android.telephony.TelephonyCallback.SignalStrengthsListener import android.telephony.TelephonyDisplayInfo import android.telephony.TelephonyManager +import com.android.systemui.log.table.Diffable +import com.android.systemui.log.table.TableRowLogger import com.android.systemui.statusbar.pipeline.mobile.data.model.DataConnectionState.Disconnected import com.android.systemui.statusbar.pipeline.shared.data.model.DataActivityModel @@ -79,4 +81,72 @@ data class MobileConnectionModel( * [TelephonyDisplayInfo.getNetworkType]. This is used to look up the proper network type icon */ val resolvedNetworkType: ResolvedNetworkType = ResolvedNetworkType.UnknownNetworkType, -) +) : Diffable { + override fun logDiffs(prevVal: MobileConnectionModel, row: TableRowLogger) { + if (prevVal.dataConnectionState != dataConnectionState) { + row.logChange(COL_CONNECTION_STATE, dataConnectionState.toString()) + } + + if (prevVal.isEmergencyOnly != isEmergencyOnly) { + row.logChange(COL_EMERGENCY, isEmergencyOnly) + } + + if (prevVal.isRoaming != isRoaming) { + row.logChange(COL_ROAMING, isRoaming) + } + + if (prevVal.operatorAlphaShort != operatorAlphaShort) { + row.logChange(COL_OPERATOR, operatorAlphaShort) + } + + if (prevVal.isGsm != isGsm) { + row.logChange(COL_IS_GSM, isGsm) + } + + if (prevVal.cdmaLevel != cdmaLevel) { + row.logChange(COL_CDMA_LEVEL, cdmaLevel) + } + + if (prevVal.primaryLevel != primaryLevel) { + row.logChange(COL_PRIMARY_LEVEL, primaryLevel) + } + + if (prevVal.dataActivityDirection != dataActivityDirection) { + row.logChange(COL_ACTIVITY_DIRECTION, dataActivityDirection.toString()) + } + + if (prevVal.carrierNetworkChangeActive != carrierNetworkChangeActive) { + row.logChange(COL_CARRIER_NETWORK_CHANGE, carrierNetworkChangeActive) + } + + if (prevVal.resolvedNetworkType != resolvedNetworkType) { + row.logChange(COL_RESOLVED_NETWORK_TYPE, resolvedNetworkType.toString()) + } + } + + override fun logFull(row: TableRowLogger) { + row.logChange(COL_CONNECTION_STATE, dataConnectionState.toString()) + row.logChange(COL_EMERGENCY, isEmergencyOnly) + row.logChange(COL_ROAMING, isRoaming) + row.logChange(COL_OPERATOR, operatorAlphaShort) + row.logChange(COL_IS_GSM, isGsm) + row.logChange(COL_CDMA_LEVEL, cdmaLevel) + row.logChange(COL_PRIMARY_LEVEL, primaryLevel) + row.logChange(COL_ACTIVITY_DIRECTION, dataActivityDirection.toString()) + row.logChange(COL_CARRIER_NETWORK_CHANGE, carrierNetworkChangeActive) + row.logChange(COL_RESOLVED_NETWORK_TYPE, resolvedNetworkType.toString()) + } + + companion object { + const val COL_EMERGENCY = "EmergencyOnly" + const val COL_ROAMING = "Roaming" + const val COL_OPERATOR = "OperatorName" + const val COL_IS_GSM = "IsGsm" + const val COL_CDMA_LEVEL = "CdmaLevel" + const val COL_PRIMARY_LEVEL = "PrimaryLevel" + const val COL_CONNECTION_STATE = "ConnectionState" + const val COL_ACTIVITY_DIRECTION = "DataActivity" + const val COL_CARRIER_NETWORK_CHANGE = "CarrierNetworkChangeActive" + const val COL_RESOLVED_NETWORK_TYPE = "NetworkType" + } +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/NetworkNameModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/NetworkNameModel.kt index a8cf35ad3029..c50d82a66c76 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/NetworkNameModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/model/NetworkNameModel.kt @@ -21,22 +21,48 @@ import android.telephony.TelephonyManager.EXTRA_DATA_SPN import android.telephony.TelephonyManager.EXTRA_PLMN import android.telephony.TelephonyManager.EXTRA_SHOW_PLMN import android.telephony.TelephonyManager.EXTRA_SHOW_SPN +import com.android.systemui.log.table.Diffable +import com.android.systemui.log.table.TableRowLogger /** * Encapsulates the data needed to show a network name for a mobile network. The data is parsed from * the intent sent by [android.telephony.TelephonyManager.ACTION_SERVICE_PROVIDERS_UPDATED]. */ -sealed interface NetworkNameModel { +sealed interface NetworkNameModel : Diffable { val name: String /** The default name is read from [com.android.internal.R.string.lockscreen_carrier_default] */ - data class Default(override val name: String) : NetworkNameModel + data class Default(override val name: String) : NetworkNameModel { + override fun logDiffs(prevVal: NetworkNameModel, row: TableRowLogger) { + if (prevVal !is Default || prevVal.name != name) { + row.logChange(COL_NETWORK_NAME, "Default($name)") + } + } + + override fun logFull(row: TableRowLogger) { + row.logChange(COL_NETWORK_NAME, "Default($name)") + } + } /** * This name has been derived from telephony intents. see * [android.telephony.TelephonyManager.ACTION_SERVICE_PROVIDERS_UPDATED] */ - data class Derived(override val name: String) : NetworkNameModel + data class Derived(override val name: String) : NetworkNameModel { + override fun logDiffs(prevVal: NetworkNameModel, row: TableRowLogger) { + if (prevVal !is Derived || prevVal.name != name) { + row.logChange(COL_NETWORK_NAME, "Derived($name)") + } + } + + override fun logFull(row: TableRowLogger) { + row.logChange(COL_NETWORK_NAME, "Derived($name)") + } + } + + companion object { + const val COL_NETWORK_NAME = "networkName" + } } fun Intent.toNetworkNameModel(separator: String): NetworkNameModel? { 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 7e9a9cea9b95..50b29f4c9028 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 @@ -36,6 +36,9 @@ import com.android.systemui.broadcast.BroadcastDispatcher import com.android.systemui.common.coroutine.ConflatedCallbackFlow.conflatedCallbackFlow import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.dagger.qualifiers.Background +import com.android.systemui.log.table.TableLogBuffer +import com.android.systemui.log.table.TableLogBufferFactory +import com.android.systemui.log.table.logDiffsForTable 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.model.ResolvedNetworkType.DefaultNetworkType @@ -46,7 +49,6 @@ import com.android.systemui.statusbar.pipeline.mobile.data.model.toNetworkNameMo import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository import com.android.systemui.statusbar.pipeline.mobile.util.MobileMappingsProxy import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger -import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logOutputChange import com.android.systemui.statusbar.pipeline.shared.data.model.toMobileDataActivityModel import com.android.systemui.util.settings.GlobalSettings import javax.inject.Inject @@ -59,6 +61,7 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach @@ -79,6 +82,7 @@ class MobileConnectionRepositoryImpl( mobileMappingsProxy: MobileMappingsProxy, bgDispatcher: CoroutineDispatcher, logger: ConnectivityPipelineLogger, + mobileLogger: TableLogBuffer, scope: CoroutineScope, ) : MobileConnectionRepository { init { @@ -95,7 +99,6 @@ class MobileConnectionRepositoryImpl( override val connectionInfo: StateFlow = run { var state = MobileConnectionModel() conflatedCallbackFlow { - // TODO (b/240569788): log all of these into the connectivity logger val callback = object : TelephonyCallback(), @@ -106,6 +109,7 @@ class MobileConnectionRepositoryImpl( TelephonyCallback.CarrierNetworkListener, TelephonyCallback.DisplayInfoListener { override fun onServiceStateChanged(serviceState: ServiceState) { + logger.logOnServiceStateChanged(serviceState, subId) state = state.copy( isEmergencyOnly = serviceState.isEmergencyOnly, @@ -116,6 +120,7 @@ class MobileConnectionRepositoryImpl( } override fun onSignalStrengthsChanged(signalStrength: SignalStrength) { + logger.logOnSignalStrengthsChanged(signalStrength, subId) val cdmaLevel = signalStrength .getCellSignalStrengths(CellSignalStrengthCdma::class.java) @@ -142,12 +147,14 @@ class MobileConnectionRepositoryImpl( dataState: Int, networkType: Int ) { + logger.logOnDataConnectionStateChanged(dataState, networkType, subId) state = state.copy(dataConnectionState = dataState.toDataConnectionType()) trySend(state) } override fun onDataActivity(direction: Int) { + logger.logOnDataActivity(direction, subId) state = state.copy( dataActivityDirection = direction.toMobileDataActivityModel() @@ -156,6 +163,7 @@ class MobileConnectionRepositoryImpl( } override fun onCarrierNetworkChange(active: Boolean) { + logger.logOnCarrierNetworkChange(active, subId) state = state.copy(carrierNetworkChangeActive = active) trySend(state) } @@ -163,6 +171,7 @@ class MobileConnectionRepositoryImpl( override fun onDisplayInfoChanged( telephonyDisplayInfo: TelephonyDisplayInfo ) { + logger.logOnDisplayInfoChanged(telephonyDisplayInfo, subId) val networkType = if (telephonyDisplayInfo.networkType == NETWORK_TYPE_UNKNOWN) { @@ -193,7 +202,11 @@ class MobileConnectionRepositoryImpl( awaitClose { telephonyManager.unregisterTelephonyCallback(callback) } } .onEach { telephonyCallbackEvent.tryEmit(Unit) } - .logOutputChange(logger, "MobileSubscriptionModel") + .logDiffsForTable( + mobileLogger, + columnPrefix = "MobileConnection ($subId)", + initialValue = state, + ) .stateIn(scope, SharingStarted.WhileSubscribed(), state) } @@ -243,19 +256,43 @@ class MobileConnectionRepositoryImpl( intent.toNetworkNameModel(networkNameSeparator) ?: defaultNetworkName } } + .distinctUntilChanged() + .logDiffsForTable( + mobileLogger, + columnPrefix = "", + initialValue = defaultNetworkName, + ) .stateIn(scope, SharingStarted.WhileSubscribed(), defaultNetworkName) - override val dataEnabled: StateFlow = + override val dataEnabled: StateFlow = run { + val initial = dataConnectionAllowed() telephonyPollingEvent .mapLatest { dataConnectionAllowed() } - .stateIn(scope, SharingStarted.WhileSubscribed(), dataConnectionAllowed()) + .distinctUntilChanged() + .logDiffsForTable( + mobileLogger, + columnPrefix = "", + columnName = "dataEnabled", + initialValue = initial, + ) + .stateIn(scope, SharingStarted.WhileSubscribed(), initial) + } private fun dataConnectionAllowed(): Boolean = telephonyManager.isDataConnectionAllowed - override val isDefaultDataSubscription: StateFlow = + override val isDefaultDataSubscription: StateFlow = run { + val initialValue = defaultDataSubId.value == subId defaultDataSubId .mapLatest { it == subId } - .stateIn(scope, SharingStarted.WhileSubscribed(), defaultDataSubId.value == subId) + .distinctUntilChanged() + .logDiffsForTable( + mobileLogger, + columnPrefix = "", + columnName = "isDefaultDataSub", + initialValue = initialValue, + ) + .stateIn(scope, SharingStarted.WhileSubscribed(), initialValue) + } class Factory @Inject @@ -266,6 +303,7 @@ class MobileConnectionRepositoryImpl( private val logger: ConnectivityPipelineLogger, private val globalSettings: GlobalSettings, private val mobileMappingsProxy: MobileMappingsProxy, + private val logFactory: TableLogBufferFactory, @Background private val bgDispatcher: CoroutineDispatcher, @Application private val scope: CoroutineScope, ) { @@ -276,6 +314,8 @@ class MobileConnectionRepositoryImpl( defaultDataSubId: StateFlow, globalMobileDataSettingChangedEvent: Flow, ): MobileConnectionRepository { + val mobileLogger = logFactory.create(tableBufferLogName(subId), 100) + return MobileConnectionRepositoryImpl( context, subId, @@ -289,8 +329,13 @@ class MobileConnectionRepositoryImpl( mobileMappingsProxy, bgDispatcher, logger, + mobileLogger, scope, ) } } + + companion object { + fun tableBufferLogName(subId: Int): String = "MobileConnectionLog [$subId]" + } } 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 a9b3d18774fd..d407abeb2315 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 @@ -51,6 +51,7 @@ import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConn import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionsRepository import com.android.systemui.statusbar.pipeline.mobile.util.MobileMappingsProxy import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger +import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logInputChange import com.android.systemui.util.settings.GlobalSettings import javax.inject.Inject import kotlinx.coroutines.CoroutineDispatcher @@ -120,6 +121,7 @@ constructor( awaitClose { subscriptionManager.removeOnSubscriptionsChangedListener(callback) } } .mapLatest { fetchSubscriptionsList().map { it.toSubscriptionModel() } } + .logInputChange(logger, "onSubscriptionsChanged") .onEach { infos -> dropUnusedReposFromCache(infos) } .stateIn(scope, started = SharingStarted.WhileSubscribed(), listOf()) @@ -136,6 +138,8 @@ constructor( telephonyManager.registerTelephonyCallback(bgDispatcher.asExecutor(), callback) awaitClose { telephonyManager.unregisterTelephonyCallback(callback) } } + .distinctUntilChanged() + .logInputChange(logger, "onActiveDataSubscriptionIdChanged") .stateIn(scope, started = SharingStarted.WhileSubscribed(), INVALID_SUBSCRIPTION_ID) private val defaultDataSubIdChangeEvent: MutableSharedFlow = @@ -149,6 +153,7 @@ constructor( intent.getIntExtra(PhoneConstants.SUBSCRIPTION_KEY, INVALID_SUBSCRIPTION_ID) } .distinctUntilChanged() + .logInputChange(logger, "ACTION_DEFAULT_DATA_SUBSCRIPTION_CHANGED") .onEach { defaultDataSubIdChangeEvent.tryEmit(Unit) } .stateIn( scope, @@ -157,13 +162,15 @@ constructor( ) private val carrierConfigChangedEvent = - broadcastDispatcher.broadcastFlow( - IntentFilter(CarrierConfigManager.ACTION_CARRIER_CONFIG_CHANGED) - ) + broadcastDispatcher + .broadcastFlow(IntentFilter(CarrierConfigManager.ACTION_CARRIER_CONFIG_CHANGED)) + .logInputChange(logger, "ACTION_CARRIER_CONFIG_CHANGED") override val defaultDataSubRatConfig: StateFlow = merge(defaultDataSubIdChangeEvent, carrierConfigChangedEvent) .mapLatest { Config.readConfig(context) } + .distinctUntilChanged() + .logInputChange(logger, "defaultDataSubRatConfig") .stateIn( scope, SharingStarted.WhileSubscribed(), @@ -171,10 +178,16 @@ constructor( ) override val defaultMobileIconMapping: Flow> = - defaultDataSubRatConfig.map { mobileMappingsProxy.mapIconSets(it) } + defaultDataSubRatConfig + .map { mobileMappingsProxy.mapIconSets(it) } + .distinctUntilChanged() + .logInputChange(logger, "defaultMobileIconMapping") override val defaultMobileIconGroup: Flow = - defaultDataSubRatConfig.map { mobileMappingsProxy.getDefaultIcons(it) } + defaultDataSubRatConfig + .map { mobileMappingsProxy.getDefaultIcons(it) } + .distinctUntilChanged() + .logInputChange(logger, "defaultMobileIconGroup") override fun getRepoForSubId(subId: Int): MobileConnectionRepository { if (!isValidSubId(subId)) { @@ -191,22 +204,24 @@ constructor( * 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) - } - } + 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 - ) + globalSettings.registerContentObserver( + globalSettings.getUriFor(MOBILE_DATA), + true, + observer + ) - awaitClose { context.contentResolver.unregisterContentObserver(observer) } - } + awaitClose { context.contentResolver.unregisterContentObserver(observer) } + } + .logInputChange(logger, "globalMobileDataSettingChangedEvent") @SuppressLint("MissingPermission") override val defaultMobileNetworkConnectivity: StateFlow = @@ -236,6 +251,8 @@ constructor( awaitClose { connectivityManager.unregisterNetworkCallback(callback) } } + .distinctUntilChanged() + .logInputChange(logger, "defaultMobileNetworkConnectivity") .stateIn(scope, SharingStarted.WhileSubscribed(), MobileConnectivityModel()) private fun isValidSubId(subId: Int): Boolean { 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 d3cf32fb44ce..d3ff3573dae2 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 @@ -18,8 +18,11 @@ package com.android.systemui.statusbar.pipeline.shared import android.net.Network import android.net.NetworkCapabilities -import com.android.systemui.log.dagger.StatusBarConnectivityLog +import android.telephony.ServiceState +import android.telephony.SignalStrength +import android.telephony.TelephonyDisplayInfo import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.log.dagger.StatusBarConnectivityLog import com.android.systemui.plugins.log.LogBuffer import com.android.systemui.plugins.log.LogLevel import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.toString @@ -28,7 +31,9 @@ import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.onEach @SysUISingleton -class ConnectivityPipelineLogger @Inject constructor( +class ConnectivityPipelineLogger +@Inject +constructor( @StatusBarConnectivityLog private val buffer: LogBuffer, ) { /** @@ -37,34 +42,23 @@ class ConnectivityPipelineLogger @Inject constructor( * Use this method for inputs that don't have any extra information besides their callback name. */ fun logInputChange(callbackName: String) { - buffer.log( - SB_LOGGING_TAG, - LogLevel.INFO, - { str1 = callbackName }, - { "Input: $str1" } - ) + buffer.log(SB_LOGGING_TAG, LogLevel.INFO, { str1 = callbackName }, { "Input: $str1" }) } - /** - * Logs a change in one of the **raw inputs** to the connectivity pipeline. - */ + /** Logs a change in one of the **raw inputs** to the connectivity pipeline. */ fun logInputChange(callbackName: String, changeInfo: String?) { buffer.log( - SB_LOGGING_TAG, - LogLevel.INFO, - { - str1 = callbackName - str2 = changeInfo - }, - { - "Input: $str1: $str2" - } + SB_LOGGING_TAG, + LogLevel.INFO, + { + str1 = callbackName + str2 = changeInfo + }, + { "Input: $str1: $str2" } ) } - /** - * Logs a **data transformation** that we performed within the connectivity pipeline. - */ + /** Logs a **data transformation** that we performed within the connectivity pipeline. */ fun logTransformation(transformationName: String, oldValue: Any?, newValue: Any?) { if (oldValue == newValue) { buffer.log( @@ -74,9 +68,7 @@ class ConnectivityPipelineLogger @Inject constructor( str1 = transformationName str2 = oldValue.toString() }, - { - "Transform: $str1: $str2 (transformation didn't change it)" - } + { "Transform: $str1: $str2 (transformation didn't change it)" } ) } else { buffer.log( @@ -87,27 +79,21 @@ class ConnectivityPipelineLogger @Inject constructor( str2 = oldValue.toString() str3 = newValue.toString() }, - { - "Transform: $str1: $str2 -> $str3" - } + { "Transform: $str1: $str2 -> $str3" } ) } } - /** - * Logs a change in one of the **outputs** to the connectivity pipeline. - */ + /** Logs a change in one of the **outputs** to the connectivity pipeline. */ fun logOutputChange(outputParamName: String, changeInfo: String) { buffer.log( - SB_LOGGING_TAG, - LogLevel.INFO, - { - str1 = outputParamName - str2 = changeInfo - }, - { - "Output: $str1: $str2" - } + SB_LOGGING_TAG, + LogLevel.INFO, + { + str1 = outputParamName + str2 = changeInfo + }, + { "Output: $str1: $str2" } ) } @@ -119,31 +105,101 @@ class ConnectivityPipelineLogger @Inject constructor( int1 = network.getNetId() str1 = networkCapabilities.toString() }, - { - "onCapabilitiesChanged: net=$int1 capabilities=$str1" - } + { "onCapabilitiesChanged: net=$int1 capabilities=$str1" } ) } fun logOnLost(network: Network) { + buffer.log( + SB_LOGGING_TAG, + LogLevel.INFO, + { int1 = network.getNetId() }, + { "onLost: net=$int1" } + ) + } + + fun logOnServiceStateChanged(serviceState: ServiceState, subId: Int) { buffer.log( SB_LOGGING_TAG, LogLevel.INFO, { - int1 = network.getNetId() + int1 = subId + bool1 = serviceState.isEmergencyOnly + bool2 = serviceState.roaming + str1 = serviceState.operatorAlphaShort }, { - "onLost: net=$int1" + "onServiceStateChanged: subId=$int1 emergencyOnly=$bool1 roaming=$bool2" + + " operator=$str1" } ) } + fun logOnSignalStrengthsChanged(signalStrength: SignalStrength, subId: Int) { + buffer.log( + SB_LOGGING_TAG, + LogLevel.INFO, + { + int1 = subId + str1 = signalStrength.toString() + }, + { "onSignalStrengthsChanged: subId=$int1 strengths=$str1" } + ) + } + + fun logOnDataConnectionStateChanged(dataState: Int, networkType: Int, subId: Int) { + buffer.log( + SB_LOGGING_TAG, + LogLevel.INFO, + { + int1 = subId + int2 = dataState + str1 = networkType.toString() + }, + { "onDataConnectionStateChanged: subId=$int1 dataState=$int2 networkType=$str1" }, + ) + } + + fun logOnDataActivity(direction: Int, subId: Int) { + buffer.log( + SB_LOGGING_TAG, + LogLevel.INFO, + { + int1 = subId + int2 = direction + }, + { "onDataActivity: subId=$int1 direction=$int2" }, + ) + } + + fun logOnCarrierNetworkChange(active: Boolean, subId: Int) { + buffer.log( + SB_LOGGING_TAG, + LogLevel.INFO, + { + int1 = subId + bool1 = active + }, + { "onCarrierNetworkChange: subId=$int1 active=$bool1" }, + ) + } + + fun logOnDisplayInfoChanged(displayInfo: TelephonyDisplayInfo, subId: Int) { + buffer.log( + SB_LOGGING_TAG, + LogLevel.INFO, + { + int1 = subId + str1 = displayInfo.toString() + }, + { "onDisplayInfoChanged: subId=$int1 displayInfo=$str1" }, + ) + } + companion object { const val SB_LOGGING_TAG = "SbConnectivity" - /** - * Log a change in one of the **inputs** to the connectivity pipeline. - */ + /** Log a change in one of the **inputs** to the connectivity pipeline. */ fun Flow.logInputChange( logger: ConnectivityPipelineLogger, inputParamName: String, @@ -155,26 +211,26 @@ class ConnectivityPipelineLogger @Inject constructor( * Log a change in one of the **inputs** to the connectivity pipeline. * * @param prettyPrint an optional function to transform the value into a readable string. - * [toString] is used if no custom function is provided. + * [toString] is used if no custom function is provided. */ fun Flow.logInputChange( logger: ConnectivityPipelineLogger, inputParamName: String, prettyPrint: (T) -> String = { it.toString() } ): Flow { - return this.onEach {logger.logInputChange(inputParamName, prettyPrint(it)) } + return this.onEach { logger.logInputChange(inputParamName, prettyPrint(it)) } } /** * Log a change in one of the **outputs** to the connectivity pipeline. * * @param prettyPrint an optional function to transform the value into a readable string. - * [toString] is used if no custom function is provided. + * [toString] is used if no custom function is provided. */ fun Flow.logOutputChange( - logger: ConnectivityPipelineLogger, - outputParamName: String, - prettyPrint: (T) -> String = { it.toString() } + logger: ConnectivityPipelineLogger, + outputParamName: String, + prettyPrint: (T) -> String = { it.toString() } ): Flow { return this.onEach { logger.logOutputChange(outputParamName, prettyPrint(it)) } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/model/MobileConnectionModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/model/MobileConnectionModelTest.kt new file mode 100644 index 000000000000..f822ba0f0a62 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/model/MobileConnectionModelTest.kt @@ -0,0 +1,93 @@ +/* + * Copyright (C) 2022 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 androidx.test.filters.SmallTest +import com.android.systemui.SysuiTestCase +import com.android.systemui.log.table.TableRowLogger +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel.Companion.COL_ACTIVITY_DIRECTION +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel.Companion.COL_CARRIER_NETWORK_CHANGE +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel.Companion.COL_CDMA_LEVEL +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel.Companion.COL_CONNECTION_STATE +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel.Companion.COL_EMERGENCY +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel.Companion.COL_IS_GSM +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel.Companion.COL_OPERATOR +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel.Companion.COL_PRIMARY_LEVEL +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel.Companion.COL_RESOLVED_NETWORK_TYPE +import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel.Companion.COL_ROAMING +import com.google.common.truth.Truth.assertThat +import org.junit.Test + +@SmallTest +class MobileConnectionModelTest : SysuiTestCase() { + + @Test + fun `log diff - initial log contains all columns`() { + val logger = TestLogger() + val connection = MobileConnectionModel() + + connection.logFull(logger) + + assertThat(logger.changes) + .contains(Pair(COL_EMERGENCY, connection.isEmergencyOnly.toString())) + assertThat(logger.changes).contains(Pair(COL_ROAMING, connection.isRoaming.toString())) + assertThat(logger.changes) + .contains(Pair(COL_OPERATOR, connection.operatorAlphaShort.toString())) + assertThat(logger.changes).contains(Pair(COL_IS_GSM, connection.isGsm.toString())) + assertThat(logger.changes).contains(Pair(COL_CDMA_LEVEL, connection.cdmaLevel.toString())) + assertThat(logger.changes) + .contains(Pair(COL_PRIMARY_LEVEL, connection.primaryLevel.toString())) + assertThat(logger.changes) + .contains(Pair(COL_CONNECTION_STATE, connection.dataConnectionState.toString())) + assertThat(logger.changes) + .contains(Pair(COL_ACTIVITY_DIRECTION, connection.dataActivityDirection.toString())) + assertThat(logger.changes) + .contains( + Pair(COL_CARRIER_NETWORK_CHANGE, connection.carrierNetworkChangeActive.toString()) + ) + assertThat(logger.changes) + .contains(Pair(COL_RESOLVED_NETWORK_TYPE, connection.resolvedNetworkType.toString())) + } + + @Test + fun `log diff - primary level changes - only level is logged`() { + val logger = TestLogger() + val connectionOld = MobileConnectionModel(primaryLevel = 1) + + val connectionNew = MobileConnectionModel(primaryLevel = 2) + + connectionNew.logDiffs(connectionOld, logger) + + assertThat(logger.changes).isEqualTo(listOf(Pair(COL_PRIMARY_LEVEL, "2"))) + } + + private class TestLogger : TableRowLogger { + val changes = mutableListOf>() + + override fun logChange(columnName: String, value: String?) { + changes.add(Pair(columnName, value.toString())) + } + + override fun logChange(columnName: String, value: Int) { + changes.add(Pair(columnName, value.toString())) + } + + override fun logChange(columnName: String, value: Boolean) { + changes.add(Pair(columnName, value.toString())) + } + } +} 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 7fa80653f29c..9a3e95826c84 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 @@ -50,6 +50,7 @@ import android.telephony.TelephonyManager.NETWORK_TYPE_LTE import android.telephony.TelephonyManager.NETWORK_TYPE_UNKNOWN import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase +import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.model.DataConnectionState import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameModel @@ -90,6 +91,7 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { @Mock private lateinit var telephonyManager: TelephonyManager @Mock private lateinit var logger: ConnectivityPipelineLogger + @Mock private lateinit var tableLogger: TableLogBuffer private val scope = CoroutineScope(IMMEDIATE) private val mobileMappings = FakeMobileMappingsProxy() @@ -116,6 +118,7 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { mobileMappings, IMMEDIATE, logger, + tableLogger, scope, ) } 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 3cc1e8b74668..b8cd7a4f6e0a 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 @@ -34,12 +34,15 @@ import com.android.internal.telephony.PhoneConstants import com.android.settingslib.R import com.android.settingslib.mobile.MobileMappings import com.android.systemui.SysuiTestCase +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.util.FakeMobileMappingsProxy import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import com.android.systemui.util.mockito.any 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 @@ -57,6 +60,7 @@ import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.mockito.ArgumentMatchers.anyInt +import org.mockito.ArgumentMatchers.anyString import org.mockito.Mock import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations @@ -72,6 +76,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { @Mock private lateinit var subscriptionManager: SubscriptionManager @Mock private lateinit var telephonyManager: TelephonyManager @Mock private lateinit var logger: ConnectivityPipelineLogger + @Mock private lateinit var logBufferFactory: TableLogBufferFactory private val mobileMappings = FakeMobileMappingsProxy() @@ -89,6 +94,10 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { } } + whenever(logBufferFactory.create(anyString(), anyInt())).thenAnswer { _ -> + mock() + } + connectionFactory = MobileConnectionRepositoryImpl.Factory( fakeBroadcastDispatcher, @@ -99,6 +108,7 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { logger = logger, mobileMappingsProxy = mobileMappings, scope = scope, + logFactory = logBufferFactory, ) underTest = @@ -270,6 +280,32 @@ class MobileConnectionsRepositoryTest : SysuiTestCase() { job.cancel() } + @Test + fun `connection repository - log buffer contains sub id in its name`() = + runBlocking(IMMEDIATE) { + val job = underTest.subscriptions.launchIn(this) + + whenever(subscriptionManager.completeActiveSubscriptionInfoList) + .thenReturn(listOf(SUB_1, SUB_2)) + getSubscriptionCallback().onSubscriptionsChanged() + + // Get repos to trigger creation + underTest.getRepoForSubId(SUB_1_ID) + verify(logBufferFactory) + .create( + eq(MobileConnectionRepositoryImpl.tableBufferLogName(SUB_1_ID)), + anyInt(), + ) + underTest.getRepoForSubId(SUB_2_ID) + verify(logBufferFactory) + .create( + eq(MobileConnectionRepositoryImpl.tableBufferLogName(SUB_2_ID)), + anyInt(), + ) + + job.cancel() + } + @Test fun testDefaultDataSubId_updatesOnBroadcast() = runBlocking(IMMEDIATE) { -- cgit v1.2.3-59-g8ed1b From 81799a75f78c2df0ea29f457b737df4d80716325 Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Fri, 16 Dec 2022 11:04:31 -0500 Subject: [Sb refactor] Mobile icon view model logging Add logging to the mobile icon view models. In order to keep the logs from duplicating, this CL also adds: 1. The reuse of a single instance of the MobileIconsViewModel (top-level), which 2. Caches the individual MobileIconViewModel common implementations on a per-subscription basis. There is now a 1-1 correspondence of the common mobile icon view model implementations and the number of subscriptions. Also updated the MobileIconViewModelTest and LocationBasedMobileIconViewModelTest classes to use the TestScope method of running tests. Test: adb logcat | grep SbConnectivity Test: tests in tests/src/com/android/systemui/statusbar/pipeline/mobile/* Bug: 238425913 Change-Id: I5157cdcfb054d4629db0f9b11da8b583760ce471 --- .../statusbar/phone/StatusBarIconController.java | 2 +- .../data/repository/MobileConnectionRepository.kt | 8 ++ .../demo/DemoMobileConnectionsRepository.kt | 19 +++- .../prod/MobileConnectionRepositoryImpl.kt | 2 + .../domain/interactor/MobileIconInteractor.kt | 6 + .../pipeline/mobile/ui/MobileUiAdapter.kt | 12 +- .../ui/viewmodel/LocationBasedMobileViewModel.kt | 41 +++++-- .../mobile/ui/viewmodel/MobileIconViewModel.kt | 123 ++++++++++++++++----- .../mobile/ui/viewmodel/MobileIconsViewModel.kt | 39 +++++-- .../repository/FakeMobileConnectionRepository.kt | 6 +- .../repository/FakeMobileConnectionsRepository.kt | 9 +- .../repository/MobileRepositorySwitcherTest.kt | 8 ++ .../demo/DemoMobileConnectionParameterizedTest.kt | 6 + .../demo/DemoMobileConnectionsRepositoryTest.kt | 7 ++ .../prod/MobileConnectionRepositoryTest.kt | 4 +- .../domain/interactor/FakeMobileIconInteractor.kt | 5 +- .../domain/interactor/FakeMobileIconsInteractor.kt | 12 +- .../domain/interactor/MobileIconInteractorTest.kt | 4 +- .../domain/interactor/MobileIconsInteractorTest.kt | 15 ++- .../LocationBasedMobileIconViewModelTest.kt | 14 ++- .../mobile/ui/viewmodel/MobileIconViewModelTest.kt | 64 +++++++---- .../ui/viewmodel/MobileIconsViewModelTest.kt | 106 ++++++++++++++++++ 22 files changed, 406 insertions(+), 106 deletions(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModelTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java index 57cdc9eed577..44e5cd9543ef 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarIconController.java @@ -387,7 +387,7 @@ public interface StatusBarIconController { if (statusBarPipelineFlags.runNewMobileIconsBackend()) { // This starts the flow for the new pipeline, and will notify us of changes if // {@link StatusBarPipelineFlags#useNewMobileIcons} is also true. - mMobileIconsViewModel = mobileUiAdapter.createMobileIconsViewModel(); + mMobileIconsViewModel = mobileUiAdapter.getMobileIconsViewModel(); MobileIconsBinder.bind(mGroup, mMobileIconsViewModel); } else { mMobileIconsViewModel = null; diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionRepository.kt index 2fd415e6777f..40e9ba1a46c7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/MobileConnectionRepository.kt @@ -20,6 +20,7 @@ import android.telephony.SubscriptionInfo import android.telephony.SubscriptionManager import android.telephony.TelephonyCallback import android.telephony.TelephonyManager +import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameModel import kotlinx.coroutines.flow.Flow @@ -39,6 +40,13 @@ import kotlinx.coroutines.flow.StateFlow interface MobileConnectionRepository { /** The subscriptionId that this connection represents */ val subId: Int + + /** + * The table log buffer created for this connection. Will have the name "MobileConnectionLog + * [subId]" + */ + val tableLogBuffer: TableLogBuffer + /** * A flow that aggregates all necessary callbacks from [TelephonyCallback] into a single * listener + model. 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 d3ee85f19347..b252de8dd389 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 @@ -24,6 +24,8 @@ import com.android.settingslib.SignalIcon import com.android.settingslib.mobile.MobileMappings import com.android.settingslib.mobile.TelephonyIcons import com.android.systemui.dagger.qualifiers.Application +import com.android.systemui.log.table.TableLogBuffer +import com.android.systemui.log.table.TableLogBufferFactory import com.android.systemui.statusbar.pipeline.mobile.data.model.DataConnectionState import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectivityModel @@ -60,6 +62,7 @@ constructor( private val dataSource: DemoModeMobileConnectionDataSource, @Application private val scope: CoroutineScope, context: Context, + private val logFactory: TableLogBufferFactory, ) : MobileConnectionsRepository { private var demoCommandJob: Job? = null @@ -149,7 +152,16 @@ constructor( override fun getRepoForSubId(subId: Int): DemoMobileConnectionRepository { return connectionRepoCache[subId] - ?: DemoMobileConnectionRepository(subId).also { connectionRepoCache[subId] = it } + ?: createDemoMobileConnectionRepo(subId).also { connectionRepoCache[subId] = it } + } + + private fun createDemoMobileConnectionRepo(subId: Int): DemoMobileConnectionRepository { + val tableLogBuffer = logFactory.create("DemoMobileConnectionLog [$subId]", 100) + + return DemoMobileConnectionRepository( + subId, + tableLogBuffer, + ) } override val globalMobileDataSettingChangedEvent = MutableStateFlow(Unit) @@ -260,7 +272,10 @@ constructor( } } -class DemoMobileConnectionRepository(override val subId: Int) : MobileConnectionRepository { +class DemoMobileConnectionRepository( + override val subId: Int, + override val tableLogBuffer: TableLogBuffer, +) : MobileConnectionRepository { override val connectionInfo = MutableStateFlow(MobileConnectionModel()) override val dataEnabled = MutableStateFlow(true) 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 50b29f4c9028..0b9e1583898e 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 @@ -96,6 +96,8 @@ class MobileConnectionRepositoryImpl( private val telephonyCallbackEvent = MutableSharedFlow(extraBufferCapacity = 1) + override val tableLogBuffer: TableLogBuffer = mobileLogger + override val connectionInfo: StateFlow = run { var state = MobileConnectionModel() conflatedCallbackFlow { 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 76e6a96a19d7..e6686dce7bbc 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 @@ -19,6 +19,7 @@ package com.android.systemui.statusbar.pipeline.mobile.domain.interactor import android.telephony.CarrierConfigManager import com.android.settingslib.SignalIcon.MobileIconGroup import com.android.systemui.dagger.qualifiers.Application +import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.model.DataConnectionState.Connected import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameModel import com.android.systemui.statusbar.pipeline.mobile.data.repository.MobileConnectionRepository @@ -35,6 +36,9 @@ import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.stateIn interface MobileIconInteractor { + /** The table log created for this connection */ + val tableLogBuffer: TableLogBuffer + /** The current mobile data activity */ val activity: Flow @@ -97,6 +101,8 @@ class MobileIconInteractorImpl( ) : MobileIconInteractor { private val connectionInfo = connectionRepository.connectionInfo + override val tableLogBuffer: TableLogBuffer = connectionRepository.tableLogBuffer + override val activity = connectionInfo.mapLatest { it.dataActivityDirection } override val isDataEnabled: StateFlow = connectionRepository.dataEnabled diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/MobileUiAdapter.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/MobileUiAdapter.kt index 62fa723dbf04..829a5cad6504 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/MobileUiAdapter.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/MobileUiAdapter.kt @@ -20,7 +20,6 @@ import com.android.systemui.CoreStartable import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.statusbar.phone.StatusBarIconController -import com.android.systemui.statusbar.phone.StatusBarIconController.IconManager import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.MobileIconsInteractor import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.MobileIconsViewModel @@ -70,6 +69,9 @@ constructor( private val mobileSubIdsState: StateFlow> = mobileSubIds.stateIn(scope, SharingStarted.WhileSubscribed(), listOf()) + /** In order to keep the logs tame, we will reuse the same top-level mobile icons view model */ + val mobileIconsViewModel = iconsViewModelFactory.create(mobileSubIdsState) + override fun start() { // Only notify the icon controller if we want to *render* the new icons. // Note that this flow may still run if @@ -81,12 +83,4 @@ constructor( } } } - - /** - * Create a MobileIconsViewModel for a given [IconManager], and bind it to to the manager's - * lifecycle. This will start collecting on [mobileSubIdsState] and link our new pipeline with - * the old view system. - */ - fun createMobileIconsViewModel(): MobileIconsViewModel = - iconsViewModelFactory.create(mobileSubIdsState) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileViewModel.kt index 070634dedd2f..b0dc41f45488 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileViewModel.kt @@ -18,7 +18,10 @@ package com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel import android.graphics.Color import com.android.systemui.statusbar.phone.StatusBarLocation +import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger +import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logOutputChange import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flowOf /** @@ -30,34 +33,50 @@ import kotlinx.coroutines.flow.flowOf */ abstract class LocationBasedMobileViewModel( val commonImpl: MobileIconViewModelCommon, + val logger: ConnectivityPipelineLogger, ) : MobileIconViewModelCommon by commonImpl { abstract val tint: Flow companion object { fun viewModelForLocation( commonImpl: MobileIconViewModelCommon, + logger: ConnectivityPipelineLogger, loc: StatusBarLocation, ): LocationBasedMobileViewModel = when (loc) { - StatusBarLocation.HOME -> HomeMobileIconViewModel(commonImpl) - StatusBarLocation.KEYGUARD -> KeyguardMobileIconViewModel(commonImpl) - StatusBarLocation.QS -> QsMobileIconViewModel(commonImpl) + StatusBarLocation.HOME -> HomeMobileIconViewModel(commonImpl, logger) + StatusBarLocation.KEYGUARD -> KeyguardMobileIconViewModel(commonImpl, logger) + StatusBarLocation.QS -> QsMobileIconViewModel(commonImpl, logger) } } } class HomeMobileIconViewModel( commonImpl: MobileIconViewModelCommon, -) : MobileIconViewModelCommon, LocationBasedMobileViewModel(commonImpl) { - override val tint: Flow = flowOf(Color.CYAN) + logger: ConnectivityPipelineLogger, +) : MobileIconViewModelCommon, LocationBasedMobileViewModel(commonImpl, logger) { + override val tint: Flow = + flowOf(Color.CYAN) + .distinctUntilChanged() + .logOutputChange(logger, "HOME tint(${commonImpl.subscriptionId})") } -class QsMobileIconViewModel(commonImpl: MobileIconViewModelCommon) : - MobileIconViewModelCommon, LocationBasedMobileViewModel(commonImpl) { - override val tint: Flow = flowOf(Color.GREEN) +class QsMobileIconViewModel( + commonImpl: MobileIconViewModelCommon, + logger: ConnectivityPipelineLogger, +) : MobileIconViewModelCommon, LocationBasedMobileViewModel(commonImpl, logger) { + override val tint: Flow = + flowOf(Color.GREEN) + .distinctUntilChanged() + .logOutputChange(logger, "QS tint(${commonImpl.subscriptionId})") } -class KeyguardMobileIconViewModel(commonImpl: MobileIconViewModelCommon) : - MobileIconViewModelCommon, LocationBasedMobileViewModel(commonImpl) { - override val tint: Flow = flowOf(Color.MAGENTA) +class KeyguardMobileIconViewModel( + commonImpl: MobileIconViewModelCommon, + logger: ConnectivityPipelineLogger, +) : MobileIconViewModelCommon, LocationBasedMobileViewModel(commonImpl, logger) { + override val tint: Flow = + flowOf(Color.MAGENTA) + .distinctUntilChanged() + .logOutputChange(logger, "KEYGUARD tint(${commonImpl.subscriptionId})") } 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 226409f66e15..2d6ac4efd512 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 @@ -16,23 +16,27 @@ package com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel -import android.graphics.Color import com.android.settingslib.graph.SignalDrawable import com.android.systemui.common.shared.model.ContentDescription import com.android.systemui.common.shared.model.Icon +import com.android.systemui.log.table.logDiffsForTable import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.MobileIconInteractor import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.MobileIconsInteractor import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger -import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logOutputChange import com.android.systemui.statusbar.pipeline.shared.data.model.DataActivityModel +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapLatest +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.stateIn /** Common interface for all of the location-based mobile icon view models. */ interface MobileIconViewModelCommon { @@ -53,7 +57,12 @@ interface MobileIconViewModelCommon { * subscription's information. * * There will be exactly one [MobileIconViewModel] per filtered subscription offered from - * [MobileIconsInteractor.filteredSubscriptions] + * [MobileIconsInteractor.filteredSubscriptions]. + * + * For the sake of keeping log spam in check, every flow funding the [MobileIconViewModelCommon] + * interface is implemented as a [StateFlow]. This ensures that each location-based mobile icon view + * model gets the exact same information, as well as allows us to log that unified state only once + * per icon. */ @Suppress("EXPERIMENTAL_IS_NOT_ENABLED") @OptIn(ExperimentalCoroutinesApi::class) @@ -63,12 +72,14 @@ constructor( iconInteractor: MobileIconInteractor, logger: ConnectivityPipelineLogger, constants: ConnectivityConstants, + scope: CoroutineScope, ) : MobileIconViewModelCommon { /** Whether or not to show the error state of [SignalDrawable] */ private val showExclamationMark: Flow = iconInteractor.isDefaultDataEnabled.mapLatest { !it } - override val iconId: Flow = + override val iconId: Flow = run { + val initial = SignalDrawable.getEmptyState(iconInteractor.numberOfLevels.value) combine(iconInteractor.level, iconInteractor.numberOfLevels, showExclamationMark) { level, numberOfLevels, @@ -76,31 +87,56 @@ constructor( SignalDrawable.getState(level, numberOfLevels, showExclamationMark) } .distinctUntilChanged() - .logOutputChange(logger, "iconId($subscriptionId)") + .logDiffsForTable( + iconInteractor.tableLogBuffer, + columnPrefix = "", + columnName = "iconId", + initialValue = initial, + ) + .stateIn(scope, SharingStarted.WhileSubscribed(), initial) + } override val networkTypeIcon: Flow = combine( - iconInteractor.networkTypeIconGroup, - iconInteractor.isDataConnected, - iconInteractor.isDataEnabled, - iconInteractor.isDefaultConnectionFailed, - iconInteractor.alwaysShowDataRatIcon, - ) { networkTypeIconGroup, dataConnected, dataEnabled, failedConnection, alwaysShow -> - val desc = - if (networkTypeIconGroup.dataContentDescription != 0) - ContentDescription.Resource(networkTypeIconGroup.dataContentDescription) - else null - val icon = Icon.Resource(networkTypeIconGroup.dataType, desc) - return@combine when { - alwaysShow -> icon - !dataConnected -> null - !dataEnabled -> null - failedConnection -> null - else -> icon + iconInteractor.networkTypeIconGroup, + iconInteractor.isDataConnected, + iconInteractor.isDataEnabled, + iconInteractor.isDefaultConnectionFailed, + iconInteractor.alwaysShowDataRatIcon, + ) { networkTypeIconGroup, dataConnected, dataEnabled, failedConnection, alwaysShow -> + val desc = + if (networkTypeIconGroup.dataContentDescription != 0) + ContentDescription.Resource(networkTypeIconGroup.dataContentDescription) + else null + val icon = Icon.Resource(networkTypeIconGroup.dataType, desc) + return@combine when { + alwaysShow -> icon + !dataConnected -> null + !dataEnabled -> null + failedConnection -> null + else -> icon + } } - } + .distinctUntilChanged() + .onEach { + // This is done as an onEach side effect since Icon is not Diffable (yet) + iconInteractor.tableLogBuffer.logChange( + prefix = "", + columnName = "networkTypeIcon", + value = it.toString(), + ) + } + .stateIn(scope, SharingStarted.WhileSubscribed(), null) - override val roaming: Flow = iconInteractor.isRoaming + override val roaming: StateFlow = + iconInteractor.isRoaming + .logDiffsForTable( + iconInteractor.tableLogBuffer, + columnPrefix = "", + columnName = "roaming", + initialValue = false, + ) + .stateIn(scope, SharingStarted.WhileSubscribed(), false) private val activity: Flow = if (!constants.shouldShowActivityConfig) { @@ -109,10 +145,39 @@ constructor( iconInteractor.activity } - override val activityInVisible: Flow = activity.map { it?.hasActivityIn ?: false } - override val activityOutVisible: Flow = activity.map { it?.hasActivityOut ?: false } - override val activityContainerVisible: Flow = - activity.map { it != null && (it.hasActivityIn || it.hasActivityOut) } + override val activityInVisible: Flow = + activity + .map { it?.hasActivityIn ?: false } + .distinctUntilChanged() + .logDiffsForTable( + iconInteractor.tableLogBuffer, + columnPrefix = "", + columnName = "activityInVisible", + initialValue = false, + ) + .stateIn(scope, SharingStarted.WhileSubscribed(), false) + + override val activityOutVisible: Flow = + activity + .map { it?.hasActivityOut ?: false } + .distinctUntilChanged() + .logDiffsForTable( + iconInteractor.tableLogBuffer, + columnPrefix = "", + columnName = "activityOutVisible", + initialValue = false, + ) + .stateIn(scope, SharingStarted.WhileSubscribed(), false) - val tint: Flow = flowOf(Color.CYAN) + override val activityContainerVisible: Flow = + activity + .map { it != null && (it.hasActivityIn || it.hasActivityOut) } + .distinctUntilChanged() + .logDiffsForTable( + iconInteractor.tableLogBuffer, + columnPrefix = "", + columnName = "activityContainerVisible", + initialValue = false, + ) + .stateIn(scope, SharingStarted.WhileSubscribed(), false) } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModel.kt index 3beb96aa8cf4..b9318b181aaf 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModel.kt @@ -14,18 +14,19 @@ * limitations under the License. */ -@file:OptIn(InternalCoroutinesApi::class) - package com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel +import androidx.annotation.VisibleForTesting +import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.statusbar.phone.StatusBarLocation import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.MobileIconsInteractor import com.android.systemui.statusbar.pipeline.mobile.ui.view.ModernStatusBarMobileView import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import javax.inject.Inject -import kotlinx.coroutines.InternalCoroutinesApi +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.launch /** * View model for describing the system's current mobile cellular connections. The result is a list @@ -39,18 +40,32 @@ constructor( private val interactor: MobileIconsInteractor, private val logger: ConnectivityPipelineLogger, private val constants: ConnectivityConstants, + @Application private val scope: CoroutineScope, ) { - /** TODO: do we need to cache these? */ + @VisibleForTesting val mobileIconSubIdCache = mutableMapOf() + + init { + scope.launch { subscriptionIdsFlow.collect { removeInvalidModelsFromCache(it) } } + } + fun viewModelForSub(subId: Int, location: StatusBarLocation): LocationBasedMobileViewModel { val common = - MobileIconViewModel( - subId, - interactor.createMobileConnectionInteractorForSubId(subId), - logger, - constants, - ) + mobileIconSubIdCache[subId] + ?: MobileIconViewModel( + subId, + interactor.createMobileConnectionInteractorForSubId(subId), + logger, + constants, + scope, + ) + .also { mobileIconSubIdCache[subId] = it } + + return LocationBasedMobileViewModel.viewModelForLocation(common, logger, location) + } - return LocationBasedMobileViewModel.viewModelForLocation(common, location) + private fun removeInvalidModelsFromCache(subIds: List) { + val subIdsToRemove = mobileIconSubIdCache.keys.filter { !subIds.contains(it) } + subIdsToRemove.forEach { mobileIconSubIdCache.remove(it) } } class Factory @@ -59,6 +74,7 @@ constructor( private val interactor: MobileIconsInteractor, private val logger: ConnectivityPipelineLogger, private val constants: ConnectivityConstants, + @Application private val scope: CoroutineScope, ) { fun create(subscriptionIdsFlow: StateFlow>): MobileIconsViewModel { return MobileIconsViewModel( @@ -66,6 +82,7 @@ constructor( interactor, logger, constants, + scope, ) } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionRepository.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionRepository.kt index 59eec5327c12..d6a9ee325b2e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionRepository.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/FakeMobileConnectionRepository.kt @@ -16,12 +16,16 @@ package com.android.systemui.statusbar.pipeline.mobile.data.repository +import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameModel import kotlinx.coroutines.flow.MutableStateFlow // TODO(b/261632894): remove this in favor of the real impl or DemoMobileConnectionRepository -class FakeMobileConnectionRepository(override val subId: Int) : MobileConnectionRepository { +class FakeMobileConnectionRepository( + override val subId: Int, + override val tableLogBuffer: TableLogBuffer, +) : MobileConnectionRepository { private val _connectionInfo = MutableStateFlow(MobileConnectionModel()) override val connectionInfo = _connectionInfo 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 04d3cdd89ab7..7f93328ee95e 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 @@ -22,14 +22,17 @@ import android.telephony.TelephonyManager import com.android.settingslib.SignalIcon import com.android.settingslib.mobile.MobileMappings import com.android.settingslib.mobile.TelephonyIcons +import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectivityModel import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel import com.android.systemui.statusbar.pipeline.mobile.util.MobileMappingsProxy import kotlinx.coroutines.flow.MutableStateFlow // TODO(b/261632894): remove this in favor of the real impl or DemoMobileConnectionsRepository -class FakeMobileConnectionsRepository(mobileMappings: MobileMappingsProxy) : - MobileConnectionsRepository { +class FakeMobileConnectionsRepository( + mobileMappings: MobileMappingsProxy, + val tableLogBuffer: TableLogBuffer, +) : MobileConnectionsRepository { val GSM_KEY = mobileMappings.toIconKey(GSM) val LTE_KEY = mobileMappings.toIconKey(LTE) val UMTS_KEY = mobileMappings.toIconKey(UMTS) @@ -63,7 +66,7 @@ class FakeMobileConnectionsRepository(mobileMappings: MobileMappingsProxy) : private val subIdRepos = mutableMapOf() override fun getRepoForSubId(subId: Int): MobileConnectionRepository { return subIdRepos[subId] - ?: FakeMobileConnectionRepository(subId).also { subIdRepos[subId] = it } + ?: FakeMobileConnectionRepository(subId, tableLogBuffer).also { subIdRepos[subId] = it } } private val _globalMobileDataSettingChangedEvent = MutableStateFlow(Unit) 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 18ae90db881a..5d377a8658a5 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 @@ -24,6 +24,8 @@ import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.demomode.DemoMode import com.android.systemui.demomode.DemoModeController +import com.android.systemui.dump.DumpManager +import com.android.systemui.log.table.TableLogBufferFactory import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel import com.android.systemui.statusbar.pipeline.mobile.data.repository.demo.DemoMobileConnectionsRepository import com.android.systemui.statusbar.pipeline.mobile.data.repository.demo.DemoModeMobileConnectionDataSource @@ -37,6 +39,7 @@ 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 import kotlinx.coroutines.Dispatchers @@ -69,12 +72,14 @@ class MobileRepositorySwitcherTest : SysuiTestCase() { private lateinit var realRepo: MobileConnectionsRepositoryImpl private lateinit var demoRepo: DemoMobileConnectionsRepository private lateinit var mockDataSource: DemoModeMobileConnectionDataSource + private lateinit var logFactory: TableLogBufferFactory @Mock private lateinit var connectivityManager: ConnectivityManager @Mock private lateinit var subscriptionManager: SubscriptionManager @Mock private lateinit var telephonyManager: TelephonyManager @Mock private lateinit var logger: ConnectivityPipelineLogger @Mock private lateinit var demoModeController: DemoModeController + @Mock private lateinit var dumpManager: DumpManager private val globalSettings = FakeSettings() private val fakeNetworkEventsFlow = MutableStateFlow(null) @@ -86,6 +91,8 @@ class MobileRepositorySwitcherTest : SysuiTestCase() { fun setUp() { MockitoAnnotations.initMocks(this) + logFactory = TableLogBufferFactory(dumpManager, FakeSystemClock()) + // Never start in demo mode whenever(demoModeController.isInDemoMode).thenReturn(false) @@ -114,6 +121,7 @@ class MobileRepositorySwitcherTest : SysuiTestCase() { dataSource = mockDataSource, scope = scope, context = context, + logFactory = logFactory, ) underTest = diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionParameterizedTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionParameterizedTest.kt index 3d5316d1f19d..210208532dd4 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionParameterizedTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionParameterizedTest.kt @@ -23,6 +23,7 @@ import androidx.test.filters.SmallTest import com.android.settingslib.SignalIcon import com.android.settingslib.mobile.TelephonyIcons import com.android.systemui.SysuiTestCase +import com.android.systemui.log.table.TableLogBufferFactory import com.android.systemui.statusbar.pipeline.mobile.data.model.DataConnectionState import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameModel @@ -30,6 +31,7 @@ import com.android.systemui.statusbar.pipeline.mobile.data.repository.demo.model import com.android.systemui.statusbar.pipeline.shared.data.model.toMobileDataActivityModel import com.android.systemui.util.mockito.mock 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.cancel @@ -54,6 +56,9 @@ import org.junit.runners.Parameterized.Parameters @RunWith(Parameterized::class) internal class DemoMobileConnectionParameterizedTest(private val testCase: TestCase) : SysuiTestCase() { + + private val logFactory = TableLogBufferFactory(mock(), FakeSystemClock()) + private val testDispatcher = UnconfinedTestDispatcher() private val testScope = TestScope(testDispatcher) @@ -76,6 +81,7 @@ internal class DemoMobileConnectionParameterizedTest(private val testCase: TestC dataSource = mockDataSource, scope = testScope.backgroundScope, context = context, + logFactory = logFactory, ) connectionsRepo.startProcessingCommands() diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepositoryTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepositoryTest.kt index 34f30eb7c0a6..cdbe75e855bc 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepositoryTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/data/repository/demo/DemoMobileConnectionsRepositoryTest.kt @@ -23,6 +23,8 @@ import androidx.test.filters.SmallTest import com.android.settingslib.SignalIcon import com.android.settingslib.mobile.TelephonyIcons.THREE_G import com.android.systemui.SysuiTestCase +import com.android.systemui.dump.DumpManager +import com.android.systemui.log.table.TableLogBufferFactory import com.android.systemui.statusbar.pipeline.mobile.data.model.DataConnectionState import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectionModel import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameModel @@ -32,6 +34,7 @@ import com.android.systemui.statusbar.pipeline.mobile.data.repository.demo.model import com.android.systemui.statusbar.pipeline.shared.data.model.toMobileDataActivityModel import com.android.systemui.util.mockito.mock import com.android.systemui.util.mockito.whenever +import com.android.systemui.util.time.FakeSystemClock import com.google.common.truth.Truth.assertThat import junit.framework.Assert import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -47,6 +50,9 @@ import org.junit.Test @OptIn(ExperimentalCoroutinesApi::class) @SmallTest class DemoMobileConnectionsRepositoryTest : SysuiTestCase() { + private val dumpManager: DumpManager = mock() + private val logFactory = TableLogBufferFactory(dumpManager, FakeSystemClock()) + private val testDispatcher = UnconfinedTestDispatcher() private val testScope = TestScope(testDispatcher) @@ -68,6 +74,7 @@ class DemoMobileConnectionsRepositoryTest : SysuiTestCase() { dataSource = mockDataSource, scope = testScope.backgroundScope, context = context, + logFactory = logFactory, ) underTest.startProcessingCommands() 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 9a3e95826c84..7970443f69b1 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 @@ -88,6 +88,7 @@ import org.mockito.MockitoAnnotations @SmallTest class MobileConnectionRepositoryTest : SysuiTestCase() { private lateinit var underTest: MobileConnectionRepositoryImpl + private lateinit var connectionsRepo: FakeMobileConnectionsRepository @Mock private lateinit var telephonyManager: TelephonyManager @Mock private lateinit var logger: ConnectivityPipelineLogger @@ -96,7 +97,6 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { private val scope = CoroutineScope(IMMEDIATE) private val mobileMappings = FakeMobileMappingsProxy() private val globalSettings = FakeSettings() - private val connectionsRepo = FakeMobileConnectionsRepository(mobileMappings) @Before fun setUp() { @@ -104,6 +104,8 @@ class MobileConnectionRepositoryTest : SysuiTestCase() { globalSettings.userId = UserHandle.USER_ALL whenever(telephonyManager.subscriptionId).thenReturn(SUB_1_ID) + connectionsRepo = FakeMobileConnectionsRepository(mobileMappings, tableLogger) + underTest = MobileConnectionRepositoryImpl( context, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconInteractor.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconInteractor.kt index c3519b7c8176..c49458909c78 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconInteractor.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/FakeMobileIconInteractor.kt @@ -19,11 +19,14 @@ package com.android.systemui.statusbar.pipeline.mobile.domain.interactor import android.telephony.CellSignalStrength import com.android.settingslib.SignalIcon import com.android.settingslib.mobile.TelephonyIcons +import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.model.NetworkNameModel import com.android.systemui.statusbar.pipeline.shared.data.model.DataActivityModel import kotlinx.coroutines.flow.MutableStateFlow -class FakeMobileIconInteractor : MobileIconInteractor { +class FakeMobileIconInteractor( + override val tableLogBuffer: TableLogBuffer, +) : MobileIconInteractor { override val alwaysShowDataRatIcon = MutableStateFlow(false) override val activity = 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 9f300e9e0cf3..19e5516b58a2 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 @@ -22,12 +22,15 @@ import android.telephony.TelephonyManager.NETWORK_TYPE_LTE import android.telephony.TelephonyManager.NETWORK_TYPE_UMTS import com.android.settingslib.SignalIcon.MobileIconGroup import com.android.settingslib.mobile.TelephonyIcons +import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel import com.android.systemui.statusbar.pipeline.mobile.util.MobileMappingsProxy -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow -class FakeMobileIconsInteractor(mobileMappings: MobileMappingsProxy) : MobileIconsInteractor { +class FakeMobileIconsInteractor( + mobileMappings: MobileMappingsProxy, + val tableLogBuffer: TableLogBuffer, +) : MobileIconsInteractor { val THREE_G_KEY = mobileMappings.toIconKey(THREE_G) val LTE_KEY = mobileMappings.toIconKey(LTE) val FOUR_G_KEY = mobileMappings.toIconKey(FOUR_G) @@ -48,8 +51,7 @@ class FakeMobileIconsInteractor(mobileMappings: MobileMappingsProxy) : MobileIco override val isDefaultConnectionFailed = MutableStateFlow(false) - private val _filteredSubscriptions = MutableStateFlow>(listOf()) - override val filteredSubscriptions: Flow> = _filteredSubscriptions + override val filteredSubscriptions = MutableStateFlow>(listOf()) private val _activeDataConnectionHasDataEnabled = MutableStateFlow(false) override val activeDataConnectionHasDataEnabled = _activeDataConnectionHasDataEnabled @@ -67,7 +69,7 @@ class FakeMobileIconsInteractor(mobileMappings: MobileMappingsProxy) : MobileIco /** Always returns a new fake interactor */ override fun createMobileConnectionInteractorForSubId(subId: Int): MobileIconInteractor { - return FakeMobileIconInteractor() + return FakeMobileIconInteractor(tableLogBuffer) } companion object { 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 4dca780425e5..83c5055a6eda 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 @@ -49,8 +49,8 @@ import org.junit.Test class MobileIconInteractorTest : SysuiTestCase() { private lateinit var underTest: MobileIconInteractor private val mobileMappingsProxy = FakeMobileMappingsProxy() - private val mobileIconsInteractor = FakeMobileIconsInteractor(mobileMappingsProxy) - private val connectionRepository = FakeMobileConnectionRepository(SUB_1_ID) + private val mobileIconsInteractor = FakeMobileIconsInteractor(mobileMappingsProxy, mock()) + private val connectionRepository = FakeMobileConnectionRepository(SUB_1_ID, mock()) private val scope = CoroutineScope(IMMEDIATE) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt index 85578942ba86..2fa3467587cc 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/domain/interactor/MobileIconsInteractorTest.kt @@ -20,6 +20,7 @@ import android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID import androidx.test.filters.SmallTest import com.android.settingslib.mobile.MobileMappings import com.android.systemui.SysuiTestCase +import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.model.MobileConnectivityModel import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel import com.android.systemui.statusbar.pipeline.mobile.data.repository.FakeMobileConnectionRepository @@ -28,6 +29,7 @@ import com.android.systemui.statusbar.pipeline.mobile.data.repository.FakeUserSe import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy import com.android.systemui.util.CarrierConfigTracker import com.android.systemui.util.mockito.whenever +import com.android.systemui.util.time.FakeSystemClock import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers @@ -44,9 +46,9 @@ import org.mockito.MockitoAnnotations @SmallTest class MobileIconsInteractorTest : SysuiTestCase() { private lateinit var underTest: MobileIconsInteractor + private lateinit var connectionsRepository: FakeMobileConnectionsRepository private val userSetupRepository = FakeUserSetupRepository() private val mobileMappingsProxy = FakeMobileMappingsProxy() - private val connectionsRepository = FakeMobileConnectionsRepository(mobileMappingsProxy) private val scope = CoroutineScope(IMMEDIATE) @Mock private lateinit var carrierConfigTracker: CarrierConfigTracker @@ -55,6 +57,7 @@ class MobileIconsInteractorTest : SysuiTestCase() { fun setUp() { MockitoAnnotations.initMocks(this) + connectionsRepository = FakeMobileConnectionsRepository(mobileMappingsProxy, tableLogBuffer) connectionsRepository.setMobileConnectionRepositoryMap( mapOf( SUB_1_ID to CONNECTION_1, @@ -290,21 +293,23 @@ class MobileIconsInteractorTest : SysuiTestCase() { companion object { private val IMMEDIATE = Dispatchers.Main.immediate + private val tableLogBuffer = + TableLogBuffer(8, "MobileIconsInteractorTest", FakeSystemClock()) private const val SUB_1_ID = 1 private val SUB_1 = SubscriptionModel(subscriptionId = SUB_1_ID) - private val CONNECTION_1 = FakeMobileConnectionRepository(SUB_1_ID) + private val CONNECTION_1 = FakeMobileConnectionRepository(SUB_1_ID, tableLogBuffer) private const val SUB_2_ID = 2 private val SUB_2 = SubscriptionModel(subscriptionId = SUB_2_ID) - private val CONNECTION_2 = FakeMobileConnectionRepository(SUB_2_ID) + private val CONNECTION_2 = FakeMobileConnectionRepository(SUB_2_ID, tableLogBuffer) private const val SUB_3_ID = 3 private val SUB_3_OPP = SubscriptionModel(subscriptionId = SUB_3_ID, isOpportunistic = true) - private val CONNECTION_3 = FakeMobileConnectionRepository(SUB_3_ID) + private val CONNECTION_3 = FakeMobileConnectionRepository(SUB_3_ID, tableLogBuffer) private const val SUB_4_ID = 4 private val SUB_4_OPP = SubscriptionModel(subscriptionId = SUB_4_ID, isOpportunistic = true) - private val CONNECTION_4 = FakeMobileConnectionRepository(SUB_4_ID) + private val CONNECTION_4 = FakeMobileConnectionRepository(SUB_4_ID, tableLogBuffer) } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt index 52232cbecf98..043d55a73076 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt @@ -19,6 +19,7 @@ package com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel import androidx.test.filters.SmallTest import com.android.settingslib.mobile.TelephonyIcons import com.android.systemui.SysuiTestCase +import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.FakeMobileIconInteractor import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.MobileIconViewModelTest.Companion.defaultSignal import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants @@ -43,9 +44,10 @@ class LocationBasedMobileIconViewModelTest : SysuiTestCase() { private lateinit var homeIcon: HomeMobileIconViewModel private lateinit var qsIcon: QsMobileIconViewModel private lateinit var keyguardIcon: KeyguardMobileIconViewModel - private val interactor = FakeMobileIconInteractor() + private lateinit var interactor: FakeMobileIconInteractor @Mock private lateinit var logger: ConnectivityPipelineLogger @Mock private lateinit var constants: ConnectivityConstants + @Mock private lateinit var tableLogBuffer: TableLogBuffer private val testDispatcher = UnconfinedTestDispatcher() private val testScope = TestScope(testDispatcher) @@ -53,6 +55,7 @@ class LocationBasedMobileIconViewModelTest : SysuiTestCase() { @Before fun setUp() { MockitoAnnotations.initMocks(this) + interactor = FakeMobileIconInteractor(tableLogBuffer) interactor.apply { setLevel(1) setIsDefaultDataEnabled(true) @@ -62,11 +65,12 @@ class LocationBasedMobileIconViewModelTest : SysuiTestCase() { setNumberOfLevels(4) isDataConnected.value = true } - commonImpl = MobileIconViewModel(SUB_1_ID, interactor, logger, constants) + commonImpl = + MobileIconViewModel(SUB_1_ID, interactor, logger, constants, testScope.backgroundScope) - homeIcon = HomeMobileIconViewModel(commonImpl) - qsIcon = QsMobileIconViewModel(commonImpl) - keyguardIcon = KeyguardMobileIconViewModel(commonImpl) + homeIcon = HomeMobileIconViewModel(commonImpl, logger) + qsIcon = QsMobileIconViewModel(commonImpl, logger) + keyguardIcon = KeyguardMobileIconViewModel(commonImpl, logger) } @Test 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 76437379e1a7..50221bc97bad 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 @@ -22,32 +22,42 @@ import com.android.settingslib.mobile.TelephonyIcons.THREE_G import com.android.systemui.SysuiTestCase import com.android.systemui.common.shared.model.ContentDescription import com.android.systemui.common.shared.model.Icon +import com.android.systemui.log.table.TableLogBuffer import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.FakeMobileIconInteractor import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import com.android.systemui.statusbar.pipeline.shared.data.model.DataActivityModel import com.android.systemui.util.mockito.whenever import com.google.common.truth.Truth.assertThat -import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest import kotlinx.coroutines.yield import org.junit.Before import org.junit.Test import org.mockito.Mock import org.mockito.MockitoAnnotations +@Suppress("EXPERIMENTAL_IS_NOT_ENABLED") +@OptIn(ExperimentalCoroutinesApi::class) @SmallTest class MobileIconViewModelTest : SysuiTestCase() { private lateinit var underTest: MobileIconViewModel - private val interactor = FakeMobileIconInteractor() + private lateinit var interactor: FakeMobileIconInteractor @Mock private lateinit var logger: ConnectivityPipelineLogger @Mock private lateinit var constants: ConnectivityConstants + @Mock private lateinit var tableLogBuffer: TableLogBuffer + + private val testDispatcher = UnconfinedTestDispatcher() + private val testScope = TestScope(testDispatcher) @Before fun setUp() { MockitoAnnotations.initMocks(this) + interactor = FakeMobileIconInteractor(tableLogBuffer) interactor.apply { setLevel(1) setIsDefaultDataEnabled(true) @@ -57,12 +67,13 @@ class MobileIconViewModelTest : SysuiTestCase() { setNumberOfLevels(4) isDataConnected.value = true } - underTest = MobileIconViewModel(SUB_1_ID, interactor, logger, constants) + underTest = + MobileIconViewModel(SUB_1_ID, interactor, logger, constants, testScope.backgroundScope) } @Test fun iconId_correctLevel_notCutout() = - runBlocking(IMMEDIATE) { + testScope.runTest { var latest: Int? = null val job = underTest.iconId.onEach { latest = it }.launchIn(this) val expected = defaultSignal() @@ -74,7 +85,7 @@ class MobileIconViewModelTest : SysuiTestCase() { @Test fun iconId_cutout_whenDefaultDataDisabled() = - runBlocking(IMMEDIATE) { + testScope.runTest { interactor.setIsDefaultDataEnabled(false) var latest: Int? = null @@ -88,7 +99,7 @@ class MobileIconViewModelTest : SysuiTestCase() { @Test fun networkType_dataEnabled_groupIsRepresented() = - runBlocking(IMMEDIATE) { + testScope.runTest { val expected = Icon.Resource( THREE_G.dataType, @@ -106,7 +117,7 @@ class MobileIconViewModelTest : SysuiTestCase() { @Test fun networkType_nullWhenDisabled() = - runBlocking(IMMEDIATE) { + testScope.runTest { interactor.setIconGroup(THREE_G) interactor.setIsDataEnabled(false) var latest: Icon? = null @@ -119,7 +130,7 @@ class MobileIconViewModelTest : SysuiTestCase() { @Test fun networkType_nullWhenFailedConnection() = - runBlocking(IMMEDIATE) { + testScope.runTest { interactor.setIconGroup(THREE_G) interactor.setIsDataEnabled(true) interactor.setIsFailedConnection(true) @@ -133,7 +144,7 @@ class MobileIconViewModelTest : SysuiTestCase() { @Test fun networkType_nullWhenDataDisconnects() = - runBlocking(IMMEDIATE) { + testScope.runTest { val initial = Icon.Resource( THREE_G.dataType, @@ -157,7 +168,7 @@ class MobileIconViewModelTest : SysuiTestCase() { @Test fun networkType_null_changeToDisabled() = - runBlocking(IMMEDIATE) { + testScope.runTest { val expected = Icon.Resource( THREE_G.dataType, @@ -180,7 +191,7 @@ class MobileIconViewModelTest : SysuiTestCase() { @Test fun networkType_alwaysShow_shownEvenWhenDisabled() = - runBlocking(IMMEDIATE) { + testScope.runTest { interactor.setIconGroup(THREE_G) interactor.setIsDataEnabled(true) interactor.alwaysShowDataRatIcon.value = true @@ -200,7 +211,7 @@ class MobileIconViewModelTest : SysuiTestCase() { @Test fun networkType_alwaysShow_shownEvenWhenDisconnected() = - runBlocking(IMMEDIATE) { + testScope.runTest { interactor.setIconGroup(THREE_G) interactor.isDataConnected.value = false interactor.alwaysShowDataRatIcon.value = true @@ -220,7 +231,7 @@ class MobileIconViewModelTest : SysuiTestCase() { @Test fun networkType_alwaysShow_shownEvenWhenFailedConnection() = - runBlocking(IMMEDIATE) { + testScope.runTest { interactor.setIconGroup(THREE_G) interactor.setIsFailedConnection(true) interactor.alwaysShowDataRatIcon.value = true @@ -240,7 +251,7 @@ class MobileIconViewModelTest : SysuiTestCase() { @Test fun roaming() = - runBlocking(IMMEDIATE) { + testScope.runTest { interactor.isRoaming.value = true var latest: Boolean? = null val job = underTest.roaming.onEach { latest = it }.launchIn(this) @@ -256,10 +267,17 @@ class MobileIconViewModelTest : SysuiTestCase() { @Test fun `data activity - null when config is off`() = - runBlocking(IMMEDIATE) { + testScope.runTest { // Create a new view model here so the constants are properly read whenever(constants.shouldShowActivityConfig).thenReturn(false) - underTest = MobileIconViewModel(SUB_1_ID, interactor, logger, constants) + underTest = + MobileIconViewModel( + SUB_1_ID, + interactor, + logger, + constants, + testScope.backgroundScope, + ) var inVisible: Boolean? = null val inJob = underTest.activityInVisible.onEach { inVisible = it }.launchIn(this) @@ -288,10 +306,17 @@ class MobileIconViewModelTest : SysuiTestCase() { @Test fun `data activity - config on - test indicators`() = - runBlocking(IMMEDIATE) { + testScope.runTest { // Create a new view model here so the constants are properly read whenever(constants.shouldShowActivityConfig).thenReturn(true) - underTest = MobileIconViewModel(SUB_1_ID, interactor, logger, constants) + underTest = + MobileIconViewModel( + SUB_1_ID, + interactor, + logger, + constants, + testScope.backgroundScope, + ) var inVisible: Boolean? = null val inJob = underTest.activityInVisible.onEach { inVisible = it }.launchIn(this) @@ -341,7 +366,6 @@ class MobileIconViewModelTest : SysuiTestCase() { } companion object { - private val IMMEDIATE = Dispatchers.Main.immediate private const val SUB_1_ID = 1 /** Convenience constructor for these tests */ diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModelTest.kt new file mode 100644 index 000000000000..d6cb76260f0b --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModelTest.kt @@ -0,0 +1,106 @@ +/* + * Copyright (C) 2022 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.ui.viewmodel + +import androidx.test.filters.SmallTest +import com.android.systemui.SysuiTestCase +import com.android.systemui.statusbar.phone.StatusBarLocation +import com.android.systemui.statusbar.pipeline.mobile.data.model.SubscriptionModel +import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.FakeMobileIconsInteractor +import com.android.systemui.statusbar.pipeline.mobile.util.FakeMobileMappingsProxy +import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants +import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger +import com.android.systemui.util.mockito.mock +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.stateIn +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.runTest +import org.junit.Before +import org.junit.Test +import org.mockito.Mock +import org.mockito.MockitoAnnotations + +@Suppress("EXPERIMENTAL_IS_NOT_ENABLED") +@OptIn(ExperimentalCoroutinesApi::class) +@SmallTest +class MobileIconsViewModelTest : SysuiTestCase() { + private lateinit var underTest: MobileIconsViewModel + private val interactor = FakeMobileIconsInteractor(FakeMobileMappingsProxy(), mock()) + + @Mock private lateinit var logger: ConnectivityPipelineLogger + @Mock private lateinit var constants: ConnectivityConstants + + private val testDispatcher = UnconfinedTestDispatcher() + private val testScope = TestScope(testDispatcher) + + @Before + fun setUp() { + MockitoAnnotations.initMocks(this) + + val subscriptionIdsFlow = + interactor.filteredSubscriptions + .map { subs -> subs.map { it.subscriptionId } } + .stateIn(testScope.backgroundScope, SharingStarted.WhileSubscribed(), listOf()) + + underTest = + MobileIconsViewModel( + subscriptionIdsFlow, + interactor, + logger, + constants, + testScope.backgroundScope, + ) + + interactor.filteredSubscriptions.value = listOf(SUB_1, SUB_2) + } + + @Test + fun `caching - mobile icon view model is reused for same sub id`() = + testScope.runTest { + val model1 = underTest.viewModelForSub(1, StatusBarLocation.HOME) + val model2 = underTest.viewModelForSub(1, StatusBarLocation.QS) + + assertThat(model1.commonImpl).isSameInstanceAs(model2.commonImpl) + } + + @Test + fun `caching - invalid view models are removed from cache when sub disappears`() = + testScope.runTest { + // Retrieve models to trigger caching + val model1 = underTest.viewModelForSub(1, StatusBarLocation.HOME) + val model2 = underTest.viewModelForSub(2, StatusBarLocation.QS) + + // Both impls are cached + assertThat(underTest.mobileIconSubIdCache) + .containsExactly(1, model1.commonImpl, 2, model2.commonImpl) + + // SUB_1 is removed from the list... + interactor.filteredSubscriptions.value = listOf(SUB_2) + + // ... and dropped from the cache + assertThat(underTest.mobileIconSubIdCache).containsExactly(2, model2.commonImpl) + } + + companion object { + private val SUB_1 = SubscriptionModel(subscriptionId = 1, isOpportunistic = false) + private val SUB_2 = SubscriptionModel(subscriptionId = 2, isOpportunistic = false) + } +} -- cgit v1.2.3-59-g8ed1b