From 8cfb8af91a5865a2eeb77ce513eceec3a1370de5 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Thu, 15 Sep 2022 22:11:51 +0000 Subject: [PATCH 1/8] [SB Refactor] Expose the wifi repository flows as StateFlows. From Evan's comment on ag/19745297, we should expose our flows as StateFlows so that it's clear to callers that it is a hot flow, not a cold flow. Bug: 238425913 Test: statusbar.pipeline tests Test: manual: Verified wifi icon via new pipeline still works Change-Id: Icec5c30f203bd11ade72f11b8fbb2b296394a693 --- .../wifi/data/repository/WifiRepository.kt | 30 ++++++++----------- .../data/repository/FakeWifiRepository.kt | 6 ++-- 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt index 103f3fc21f918..f41264fc570ff 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt @@ -43,24 +43,18 @@ import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.channels.awaitClose -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.stateIn -/** - * Provides data related to the wifi state. - */ +/** Provides data related to the wifi state. */ interface WifiRepository { - /** - * Observable for the current wifi network. - */ - val wifiNetwork: Flow + /** Observable for the current wifi network. */ + val wifiNetwork: StateFlow - /** - * Observable for the current wifi network activity. - */ - val wifiActivity: Flow + /** Observable for the current wifi network activity. */ + val wifiActivity: StateFlow } /** Real implementation of [WifiRepository]. */ @@ -74,7 +68,7 @@ class WifiRepositoryImpl @Inject constructor( @Application scope: CoroutineScope, wifiManager: WifiManager?, ) : WifiRepository { - override val wifiNetwork: Flow = conflatedCallbackFlow { + override val wifiNetwork: StateFlow = conflatedCallbackFlow { var currentWifi: WifiNetworkModel = WIFI_NETWORK_DEFAULT val callback = object : ConnectivityManager.NetworkCallback(FLAG_INCLUDE_LOCATION_INFO) { @@ -132,7 +126,7 @@ class WifiRepositoryImpl @Inject constructor( initialValue = WIFI_NETWORK_DEFAULT ) - override val wifiActivity: Flow = + override val wifiActivity: StateFlow = if (wifiManager == null) { Log.w(SB_LOGGING_TAG, "Null WifiManager; skipping activity callback") flowOf(ACTIVITY_DEFAULT) @@ -142,13 +136,15 @@ class WifiRepositoryImpl @Inject constructor( logger.logInputChange("onTrafficStateChange", prettyPrintActivity(state)) trySend(trafficStateToWifiActivityModel(state)) } - - trySend(ACTIVITY_DEFAULT) wifiManager.registerTrafficStateCallback(mainExecutor, callback) - awaitClose { wifiManager.unregisterTrafficStateCallback(callback) } } } + .stateIn( + scope, + started = SharingStarted.WhileSubscribed(), + initialValue = ACTIVITY_DEFAULT + ) companion object { val ACTIVITY_DEFAULT = WifiActivityModel(hasActivityIn = false, hasActivityOut = false) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt index 6b8d4aa7c51f4..d59a25b8c5a9a 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt @@ -19,17 +19,17 @@ package com.android.systemui.statusbar.pipeline.wifi.data.repository import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiActivityModel import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepositoryImpl.Companion.ACTIVITY_DEFAULT -import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow /** Fake implementation of [WifiRepository] exposing set methods for all the flows. */ class FakeWifiRepository : WifiRepository { private val _wifiNetwork: MutableStateFlow = MutableStateFlow(WifiNetworkModel.Inactive) - override val wifiNetwork: Flow = _wifiNetwork + override val wifiNetwork: StateFlow = _wifiNetwork private val _wifiActivity = MutableStateFlow(ACTIVITY_DEFAULT) - override val wifiActivity: Flow = _wifiActivity + override val wifiActivity: StateFlow = _wifiActivity fun setWifiNetwork(wifiNetworkModel: WifiNetworkModel) { _wifiNetwork.value = wifiNetworkModel From 4513768e3be8fe8f10771f233b50eb019a7a95bd Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Wed, 14 Sep 2022 21:22:18 +0000 Subject: [PATCH 2/8] [SB Refactor] Display the activity in and out icons using the new pipeline. Bug: 238425913 Test: manual: Verified activity icons show and hide as the actual activity changes (see video in b/238425913#comment28) Test: statusbar.pipeline tests Change-Id: I454555db129fce0e7e168d55148c304ba43122c0 --- .../shared/ConnectivityPipelineLogger.kt | 2 +- .../wifi/data/repository/WifiRepository.kt | 2 +- .../wifi/domain/interactor/WifiInteractor.kt | 18 +- .../model/WifiActivityModel.kt | 6 +- .../pipeline/wifi/ui/binder/WifiViewBinder.kt | 32 +- .../wifi/ui/viewmodel/WifiViewModel.kt | 49 ++- .../shared/ConnectivityPipelineLoggerTest.kt | 42 ++- .../data/repository/FakeWifiRepository.kt | 2 +- .../data/repository/WifiRepositoryImplTest.kt | 2 +- .../domain/interactor/WifiInteractorTest.kt | 239 ++++++-------- .../wifi/ui/viewmodel/WifiViewModelTest.kt | 302 +++++++++++++++--- 11 files changed, 473 insertions(+), 223 deletions(-) rename packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/{data => shared}/model/WifiActivityModel.kt (86%) 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 88d8a86d39f2d..3a7ff167af18d 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 @@ -133,7 +133,7 @@ class ConnectivityPipelineLogger @Inject constructor( * @param prettyPrint an optional function to transform the value into a readable string. * [toString] is used if no custom function is provided. */ - fun Flow.logOutputChange( + fun Flow.logOutputChange( logger: ConnectivityPipelineLogger, outputParamName: String, prettyPrint: (T) -> String = { it.toString() } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt index f41264fc570ff..6f20cbc004e1a 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt @@ -36,8 +36,8 @@ import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.SB_LOGGING_TAG -import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiActivityModel import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel +import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiActivityModel import java.util.concurrent.Executor import javax.inject.Inject import kotlinx.coroutines.CoroutineScope diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractor.kt index 952525d243f99..ce6003f0abd9b 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractor.kt @@ -22,9 +22,10 @@ import com.android.systemui.statusbar.pipeline.shared.data.model.ConnectivitySlo import com.android.systemui.statusbar.pipeline.shared.data.repository.ConnectivityRepository import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepository +import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiActivityModel import javax.inject.Inject import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.map /** @@ -38,7 +39,11 @@ class WifiInteractor @Inject constructor( connectivityRepository: ConnectivityRepository, wifiRepository: WifiRepository, ) { - private val ssid: Flow = wifiRepository.wifiNetwork.map { info -> + /** + * The SSID (service set identifier) of the wifi network. Null if we don't have a network, or + * have a network but no valid SSID. + */ + val ssid: Flow = wifiRepository.wifiNetwork.map { info -> when (info) { is WifiNetworkModel.Inactive -> null is WifiNetworkModel.CarrierMerged -> null @@ -54,14 +59,11 @@ class WifiInteractor @Inject constructor( /** Our current wifi network. See [WifiNetworkModel]. */ val wifiNetwork: Flow = wifiRepository.wifiNetwork + /** Our current wifi activity. See [WifiActivityModel]. */ + val activity: StateFlow = wifiRepository.wifiActivity + /** True if we're configured to force-hide the wifi icon and false otherwise. */ val isForceHidden: Flow = connectivityRepository.forceHiddenSlots.map { it.contains(ConnectivitySlot.WIFI) } - - /** True if our wifi network has activity in (download), and false otherwise. */ - val hasActivityIn: Flow = - combine(wifiRepository.wifiActivity, ssid) { activity, ssid -> - activity.hasActivityIn && ssid != null - } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/model/WifiActivityModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiActivityModel.kt similarity index 86% rename from packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/model/WifiActivityModel.kt rename to packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiActivityModel.kt index 44c04968041e3..574610605b4e2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/model/WifiActivityModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/shared/model/WifiActivityModel.kt @@ -14,11 +14,9 @@ * limitations under the License. */ -package com.android.systemui.statusbar.pipeline.wifi.data.model +package com.android.systemui.statusbar.pipeline.wifi.shared.model -/** - * Provides information on the current wifi activity. - */ +/** Provides information on the current wifi activity. */ data class WifiActivityModel( /** True if the wifi has activity in (download). */ val hasActivityIn: Boolean, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt index 4fad3274d12f5..26667ab6d413c 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt @@ -48,6 +48,9 @@ object WifiViewBinder { viewModel: WifiViewModel, ) { val iconView = view.requireViewById(R.id.wifi_signal) + val activityInView = view.requireViewById(R.id.wifi_in) + val activityOutView = view.requireViewById(R.id.wifi_out) + val activityContainerView = view.requireViewById(R.id.inout_container) view.isVisible = true iconView.isVisible = true @@ -61,20 +64,37 @@ object WifiViewBinder { // [ModernStatusBarWifiView.isIconVisible], which is what actually makes // the view GONE. view.isVisible = wifiIcon != null - wifiIcon?.let { - IconViewBinder.bind(wifiIcon, iconView) - } + wifiIcon?.let { IconViewBinder.bind(wifiIcon, iconView) } } } launch { viewModel.tint.collect { tint -> - iconView.imageTintList = ColorStateList.valueOf(tint) + val tintList = ColorStateList.valueOf(tint) + iconView.imageTintList = tintList + activityInView.imageTintList = tintList + activityOutView.imageTintList = tintList + } + } + + launch { + viewModel.isActivityInViewVisible.distinctUntilChanged().collect { visible -> + activityInView.isVisible = visible + } + } + + launch { + viewModel.isActivityOutViewVisible.distinctUntilChanged().collect { visible -> + activityOutView.isVisible = visible + } + } + + launch { + viewModel.isActivityContainerVisible.distinctUntilChanged().collect { visible -> + activityContainerView.isVisible = visible } } } } - - // TODO(b/238425913): Hook up to [viewModel] to render actual changes to the wifi icon. } } 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 3c243ac908311..8197e89cb93ef 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 @@ -35,9 +35,11 @@ import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.domain.interactor.WifiInteractor import com.android.systemui.statusbar.pipeline.wifi.shared.WifiConstants +import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiActivityModel import javax.inject.Inject import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map @@ -46,11 +48,11 @@ import kotlinx.coroutines.flow.map * Models the UI state for the status bar wifi icon. */ class WifiViewModel @Inject constructor( - statusBarPipelineFlags: StatusBarPipelineFlags, - private val constants: WifiConstants, + constants: WifiConstants, private val context: Context, - private val logger: ConnectivityPipelineLogger, - private val interactor: WifiInteractor, + logger: ConnectivityPipelineLogger, + interactor: WifiInteractor, + statusBarPipelineFlags: StatusBarPipelineFlags, ) { /** * The drawable resource ID to use for the wifi icon. Null if we shouldn't display any icon. @@ -109,17 +111,36 @@ class WifiViewModel @Inject constructor( } } - /** - * True if the activity in icon should be displayed and false otherwise. - */ - val isActivityInVisible: Flow - get() = - if (!constants.shouldShowActivityConfig) { - flowOf(false) - } else { - interactor.hasActivityIn + /** The wifi activity status. Null if we shouldn't display the activity status. */ + private val activity: Flow = + if (!constants.shouldShowActivityConfig) { + flowOf(null) + } else { + combine(interactor.activity, interactor.ssid) { activity, ssid -> + when (ssid) { + null -> null + else -> activity + } } - .logOutputChange(logger, "activityInVisible") + } + .distinctUntilChanged() + .logOutputChange(logger, "activity") + + /** True if the activity in view should be visible. */ + val isActivityInViewVisible: Flow = activity.map { it?.hasActivityIn == true } + + /** True if the activity out view should be visible. */ + val isActivityOutViewVisible: Flow = activity.map { it?.hasActivityOut == true } + + /** True if the activity container view should be visible. */ + val isActivityContainerVisible: Flow = + combine(isActivityInViewVisible, isActivityOutViewVisible) { activityIn, activityOut -> + activityIn || activityOut + } + + // TODO(b/238425913): Update this class to use state flows instead. Right now, we have a ton of + // duplicate activity logs because the cold flows are getting duplicated for the three + // activityVisible flows. /** The tint that should be applied to the icon. */ val tint: Flow = if (!statusBarPipelineFlags.useNewPipelineDebugColoring()) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLoggerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLoggerTest.kt index 36be1be309d6d..d3d8d542e0785 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLoggerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLoggerTest.kt @@ -23,9 +23,15 @@ import com.android.systemui.SysuiTestCase import com.android.systemui.dump.DumpManager import com.android.systemui.log.LogBufferFactory import com.android.systemui.log.LogcatEchoTracker +import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logOutputChange import com.google.common.truth.Truth.assertThat import java.io.PrintWriter import java.io.StringWriter +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.runBlocking import org.junit.Test import org.mockito.Mockito import org.mockito.Mockito.mock @@ -64,12 +70,34 @@ class ConnectivityPipelineLoggerTest : SysuiTestCase() { assertThat(actualString).contains(expectedNetId) } - private val NET_1_ID = 100 - private val NET_1 = com.android.systemui.util.mockito.mock().also { - Mockito.`when`(it.getNetId()).thenReturn(NET_1_ID) + @Test + fun logOutputChange_printsValuesAndNulls() = runBlocking(IMMEDIATE) { + val flow: Flow = flowOf(1, null, 3) + + val job = flow + .logOutputChange(logger, "testInts") + .launchIn(this) + + val stringWriter = StringWriter() + buffer.dump(PrintWriter(stringWriter), tailLength = 0) + val actualString = stringWriter.toString() + + assertThat(actualString).contains("1") + assertThat(actualString).contains("null") + assertThat(actualString).contains("3") + + job.cancel() + } + + companion object { + private const val NET_1_ID = 100 + private val NET_1 = com.android.systemui.util.mockito.mock().also { + Mockito.`when`(it.getNetId()).thenReturn(NET_1_ID) + } + private val NET_1_CAPS = NetworkCapabilities.Builder() + .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) + .addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) + .build() + private val IMMEDIATE = Dispatchers.Main.immediate } - private val NET_1_CAPS = NetworkCapabilities.Builder() - .addTransportType(NetworkCapabilities.TRANSPORT_CELLULAR) - .addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED) - .build() } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt index d59a25b8c5a9a..cd0f27a25b7e2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt @@ -16,9 +16,9 @@ package com.android.systemui.statusbar.pipeline.wifi.data.repository -import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiActivityModel import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepositoryImpl.Companion.ACTIVITY_DEFAULT +import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiActivityModel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepositoryImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepositoryImplTest.kt index d070ba0e47beb..1878ce5b7e17b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepositoryImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepositoryImplTest.kt @@ -29,10 +29,10 @@ import android.net.wifi.WifiManager.TrafficStateCallback import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger -import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiActivityModel import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepositoryImpl.Companion.ACTIVITY_DEFAULT import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepositoryImpl.Companion.WIFI_NETWORK_DEFAULT +import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiActivityModel import com.android.systemui.util.concurrency.FakeExecutor import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.argumentCaptor diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractorTest.kt index e896749d9a94a..622f20769f0df 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractorTest.kt @@ -16,13 +16,14 @@ package com.android.systemui.statusbar.pipeline.wifi.domain.interactor +import android.net.wifi.WifiManager import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.statusbar.pipeline.shared.data.model.ConnectivitySlot import com.android.systemui.statusbar.pipeline.shared.data.repository.FakeConnectivityRepository -import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiActivityModel import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.data.repository.FakeWifiRepository +import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiActivityModel import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi @@ -50,171 +51,105 @@ class WifiInteractorTest : SysuiTestCase() { } @Test - fun hasActivityIn_noInOrOut_outputsFalse() = runBlocking(IMMEDIATE) { - wifiRepository.setWifiNetwork(VALID_WIFI_NETWORK_MODEL) - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = false, hasActivityOut = false) - ) - - var latest: Boolean? = null - val job = underTest - .hasActivityIn - .onEach { latest = it } - .launchIn(this) - - assertThat(latest).isFalse() - - job.cancel() - } - - @Test - fun hasActivityIn_onlyOut_outputsFalse() = runBlocking(IMMEDIATE) { - wifiRepository.setWifiNetwork(VALID_WIFI_NETWORK_MODEL) - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = false, hasActivityOut = true) - ) - - var latest: Boolean? = null - val job = underTest - .hasActivityIn - .onEach { latest = it } - .launchIn(this) - - assertThat(latest).isFalse() - - job.cancel() - } - - @Test - fun hasActivityIn_onlyIn_outputsTrue() = runBlocking(IMMEDIATE) { - wifiRepository.setWifiNetwork(VALID_WIFI_NETWORK_MODEL) - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = true, hasActivityOut = false) - ) - - var latest: Boolean? = null - val job = underTest - .hasActivityIn - .onEach { latest = it } - .launchIn(this) - - assertThat(latest).isTrue() - - job.cancel() - } - - @Test - fun hasActivityIn_inAndOut_outputsTrue() = runBlocking(IMMEDIATE) { - wifiRepository.setWifiNetwork(VALID_WIFI_NETWORK_MODEL) - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = true, hasActivityOut = true) - ) - - var latest: Boolean? = null - val job = underTest - .hasActivityIn - .onEach { latest = it } - .launchIn(this) - - assertThat(latest).isTrue() - - job.cancel() - } - - @Test - fun hasActivityIn_ssidNull_outputsFalse() = runBlocking(IMMEDIATE) { - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(networkId = 1, ssid = null)) - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = true, hasActivityOut = true) - ) - - var latest: Boolean? = null - val job = underTest - .hasActivityIn - .onEach { latest = it } - .launchIn(this) - - assertThat(latest).isFalse() - - job.cancel() - } - - @Test - fun hasActivityIn_inactiveNetwork_outputsFalse() = runBlocking(IMMEDIATE) { + fun ssid_inactiveNetwork_outputsNull() = runBlocking(IMMEDIATE) { wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive) - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = true, hasActivityOut = true) - ) - var latest: Boolean? = null + var latest: String? = "default" val job = underTest - .hasActivityIn + .ssid .onEach { latest = it } .launchIn(this) - assertThat(latest).isFalse() + assertThat(latest).isNull() job.cancel() } @Test - fun hasActivityIn_carrierMergedNetwork_outputsFalse() = runBlocking(IMMEDIATE) { + fun ssid_carrierMergedNetwork_outputsNull() = runBlocking(IMMEDIATE) { wifiRepository.setWifiNetwork(WifiNetworkModel.CarrierMerged) - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = true, hasActivityOut = true) - ) - var latest: Boolean? = null + var latest: String? = "default" val job = underTest - .hasActivityIn + .ssid .onEach { latest = it } .launchIn(this) - assertThat(latest).isFalse() + assertThat(latest).isNull() job.cancel() } @Test - fun hasActivityIn_multipleChanges_multipleOutputChanges() = runBlocking(IMMEDIATE) { - wifiRepository.setWifiNetwork(VALID_WIFI_NETWORK_MODEL) + fun ssid_isPasspointAccessPoint_outputsPasspointName() = runBlocking(IMMEDIATE) { + wifiRepository.setWifiNetwork(WifiNetworkModel.Active( + networkId = 1, + isPasspointAccessPoint = true, + passpointProviderFriendlyName = "friendly", + )) - var latest: Boolean? = null + var latest: String? = null val job = underTest - .hasActivityIn - .onEach { latest = it } - .launchIn(this) + .ssid + .onEach { latest = it } + .launchIn(this) - // Conduct a series of changes and verify we catch each of them in succession - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = true, hasActivityOut = false) - ) - yield() - assertThat(latest).isTrue() + assertThat(latest).isEqualTo("friendly") - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = false, hasActivityOut = true) - ) - yield() - assertThat(latest).isFalse() + job.cancel() + } - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = true, hasActivityOut = true) - ) - yield() - assertThat(latest).isTrue() + @Test + fun ssid_isOnlineSignUpForPasspoint_outputsPasspointName() = runBlocking(IMMEDIATE) { + wifiRepository.setWifiNetwork(WifiNetworkModel.Active( + networkId = 1, + isOnlineSignUpForPasspointAccessPoint = true, + passpointProviderFriendlyName = "friendly", + )) - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = true, hasActivityOut = false) - ) - yield() - assertThat(latest).isTrue() + var latest: String? = null + val job = underTest + .ssid + .onEach { latest = it } + .launchIn(this) - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = false, hasActivityOut = false) - ) - yield() - assertThat(latest).isFalse() + assertThat(latest).isEqualTo("friendly") + + job.cancel() + } + + @Test + fun ssid_unknownSsid_outputsNull() = runBlocking(IMMEDIATE) { + wifiRepository.setWifiNetwork(WifiNetworkModel.Active( + networkId = 1, + ssid = WifiManager.UNKNOWN_SSID, + )) + + var latest: String? = "default" + val job = underTest + .ssid + .onEach { latest = it } + .launchIn(this) + + assertThat(latest).isNull() + + job.cancel() + } + + @Test + fun ssid_validSsid_outputsSsid() = runBlocking(IMMEDIATE) { + wifiRepository.setWifiNetwork(WifiNetworkModel.Active( + networkId = 1, + ssid = "MyAwesomeWifiNetwork", + )) + + var latest: String? = null + val job = underTest + .ssid + .onEach { latest = it } + .launchIn(this) + + assertThat(latest).isEqualTo("MyAwesomeWifiNetwork") job.cancel() } @@ -241,6 +176,32 @@ class WifiInteractorTest : SysuiTestCase() { job.cancel() } + @Test + fun activity_matchesRepoWifiActivity() = runBlocking(IMMEDIATE) { + var latest: WifiActivityModel? = null + val job = underTest + .activity + .onEach { latest = it } + .launchIn(this) + + val activity1 = WifiActivityModel(hasActivityIn = true, hasActivityOut = true) + wifiRepository.setWifiActivity(activity1) + yield() + assertThat(latest).isEqualTo(activity1) + + val activity2 = WifiActivityModel(hasActivityIn = false, hasActivityOut = false) + wifiRepository.setWifiActivity(activity2) + yield() + assertThat(latest).isEqualTo(activity2) + + val activity3 = WifiActivityModel(hasActivityIn = true, hasActivityOut = false) + wifiRepository.setWifiActivity(activity3) + yield() + assertThat(latest).isEqualTo(activity3) + + job.cancel() + } + @Test fun isForceHidden_repoHasWifiHidden_outputsTrue() = runBlocking(IMMEDIATE) { connectivityRepository.setForceHiddenIcons(setOf(ConnectivitySlot.WIFI)) @@ -270,10 +231,6 @@ class WifiInteractorTest : SysuiTestCase() { job.cancel() } - - companion object { - val VALID_WIFI_NETWORK_MODEL = WifiNetworkModel.Active(networkId = 1, ssid = "AB") - } } private val IMMEDIATE = Dispatchers.Main.immediate 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 43103a065e68c..f0ef9d043169c 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 @@ -29,11 +29,11 @@ import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import com.android.systemui.statusbar.pipeline.shared.data.model.ConnectivitySlot import com.android.systemui.statusbar.pipeline.shared.data.repository.FakeConnectivityRepository -import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiActivityModel import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.data.repository.FakeWifiRepository import com.android.systemui.statusbar.pipeline.wifi.domain.interactor.WifiInteractor import com.android.systemui.statusbar.pipeline.wifi.shared.WifiConstants +import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiActivityModel import com.android.systemui.statusbar.pipeline.wifi.ui.viewmodel.WifiViewModel.Companion.NO_INTERNET import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.Dispatchers @@ -67,14 +67,7 @@ class WifiViewModelTest : SysuiTestCase() { connectivityRepository = FakeConnectivityRepository() wifiRepository = FakeWifiRepository() interactor = WifiInteractor(connectivityRepository, wifiRepository) - - underTest = WifiViewModel( - statusBarPipelineFlags, - constants, - context, - logger, - interactor - ) + createAndSetViewModel() } @Test @@ -219,68 +212,299 @@ class WifiViewModelTest : SysuiTestCase() { } @Test - fun activityInVisible_showActivityConfigFalse_outputsFalse() = runBlocking(IMMEDIATE) { + fun activity_showActivityConfigFalse_outputsFalse() = runBlocking(IMMEDIATE) { whenever(constants.shouldShowActivityConfig).thenReturn(false) + createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) - var latest: Boolean? = null - val job = underTest - .isActivityInVisible - .onEach { latest = it } + var activityIn: Boolean? = null + val activityInJob = underTest + .isActivityInViewVisible + .onEach { activityIn = it } .launchIn(this) - // Verify that on launch, we receive a false. - assertThat(latest).isFalse() + var activityOut: Boolean? = null + val activityOutJob = underTest + .isActivityOutViewVisible + .onEach { activityOut = it } + .launchIn(this) - job.cancel() + var activityContainer: Boolean? = null + val activityContainerJob = underTest + .isActivityContainerVisible + .onEach { activityContainer = it } + .launchIn(this) + + // Verify that on launch, we receive false. + assertThat(activityIn).isFalse() + assertThat(activityOut).isFalse() + assertThat(activityContainer).isFalse() + + activityInJob.cancel() + activityOutJob.cancel() + activityContainerJob.cancel() } @Test - fun activityInVisible_showActivityConfigFalse_noUpdatesReceived() = runBlocking(IMMEDIATE) { + fun activity_showActivityConfigFalse_noUpdatesReceived() = runBlocking(IMMEDIATE) { whenever(constants.shouldShowActivityConfig).thenReturn(false) + createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) - var latest: Boolean? = null - val job = underTest - .isActivityInVisible - .onEach { latest = it } - .launchIn(this) + var activityIn: Boolean? = null + val activityInJob = underTest + .isActivityInViewVisible + .onEach { activityIn = it } + .launchIn(this) - // Update the repo to have activityIn - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = true, hasActivityOut = false) - ) + var activityOut: Boolean? = null + val activityOutJob = underTest + .isActivityOutViewVisible + .onEach { activityOut = it } + .launchIn(this) + + var activityContainer: Boolean? = null + val activityContainerJob = underTest + .isActivityContainerVisible + .onEach { activityContainer = it } + .launchIn(this) + + // WHEN we update the repo to have activity + val activity = WifiActivityModel(hasActivityIn = true, hasActivityOut = true) + wifiRepository.setWifiActivity(activity) yield() - // Verify that we didn't update to activityIn=true (because our config is false) - assertThat(latest).isFalse() + // THEN we didn't update to the new activity (because our config is false) + assertThat(activityIn).isFalse() + assertThat(activityOut).isFalse() + assertThat(activityContainer).isFalse() - job.cancel() + activityInJob.cancel() + activityOutJob.cancel() + activityContainerJob.cancel() } @Test - fun activityInVisible_showActivityConfigTrue_outputsUpdate() = runBlocking(IMMEDIATE) { + fun activity_nullSsid_outputsFalse() = runBlocking(IMMEDIATE) { whenever(constants.shouldShowActivityConfig).thenReturn(true) + createAndSetViewModel() + + wifiRepository.setWifiNetwork(WifiNetworkModel.Active(NETWORK_ID, ssid = null)) + + var activityIn: Boolean? = null + val activityInJob = underTest + .isActivityInViewVisible + .onEach { activityIn = it } + .launchIn(this) + + var activityOut: Boolean? = null + val activityOutJob = underTest + .isActivityOutViewVisible + .onEach { activityOut = it } + .launchIn(this) + + var activityContainer: Boolean? = null + val activityContainerJob = underTest + .isActivityContainerVisible + .onEach { activityContainer = it } + .launchIn(this) + + // WHEN we update the repo to have activity + val activity = WifiActivityModel(hasActivityIn = true, hasActivityOut = true) + wifiRepository.setWifiActivity(activity) + yield() + + // THEN we still output false because our network's SSID is null + assertThat(activityIn).isFalse() + assertThat(activityOut).isFalse() + assertThat(activityContainer).isFalse() + + activityInJob.cancel() + activityOutJob.cancel() + activityContainerJob.cancel() + } + + @Test + fun activityIn_hasActivityInTrue_outputsTrue() = runBlocking(IMMEDIATE) { + whenever(constants.shouldShowActivityConfig).thenReturn(true) + createAndSetViewModel() wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) var latest: Boolean? = null val job = underTest - .isActivityInVisible - .onEach { latest = it } - .launchIn(this) + .isActivityInViewVisible + .onEach { latest = it } + .launchIn(this) - // Update the repo to have activityIn - wifiRepository.setWifiActivity( - WifiActivityModel(hasActivityIn = true, hasActivityOut = false) - ) + val activity = WifiActivityModel(hasActivityIn = true, hasActivityOut = false) + wifiRepository.setWifiActivity(activity) yield() - // Verify that we updated to activityIn=true assertThat(latest).isTrue() job.cancel() } + @Test + fun activityIn_hasActivityInFalse_outputsFalse() = runBlocking(IMMEDIATE) { + whenever(constants.shouldShowActivityConfig).thenReturn(true) + createAndSetViewModel() + wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) + + var latest: Boolean? = null + val job = underTest + .isActivityInViewVisible + .onEach { latest = it } + .launchIn(this) + + val activity = WifiActivityModel(hasActivityIn = false, hasActivityOut = true) + wifiRepository.setWifiActivity(activity) + yield() + + assertThat(latest).isFalse() + + job.cancel() + } + + @Test + fun activityOut_hasActivityOutTrue_outputsTrue() = runBlocking(IMMEDIATE) { + whenever(constants.shouldShowActivityConfig).thenReturn(true) + createAndSetViewModel() + wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) + + var latest: Boolean? = null + val job = underTest + .isActivityOutViewVisible + .onEach { latest = it } + .launchIn(this) + + val activity = WifiActivityModel(hasActivityIn = false, hasActivityOut = true) + wifiRepository.setWifiActivity(activity) + yield() + + assertThat(latest).isTrue() + + job.cancel() + } + + @Test + fun activityOut_hasActivityOutFalse_outputsFalse() = runBlocking(IMMEDIATE) { + whenever(constants.shouldShowActivityConfig).thenReturn(true) + createAndSetViewModel() + wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) + + var latest: Boolean? = null + val job = underTest + .isActivityOutViewVisible + .onEach { latest = it } + .launchIn(this) + + val activity = WifiActivityModel(hasActivityIn = true, hasActivityOut = false) + wifiRepository.setWifiActivity(activity) + yield() + + assertThat(latest).isFalse() + + job.cancel() + } + + @Test + fun activityContainer_hasActivityInTrue_outputsTrue() = runBlocking(IMMEDIATE) { + whenever(constants.shouldShowActivityConfig).thenReturn(true) + createAndSetViewModel() + wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) + + var latest: Boolean? = null + val job = underTest + .isActivityContainerVisible + .onEach { latest = it } + .launchIn(this) + + val activity = WifiActivityModel(hasActivityIn = true, hasActivityOut = false) + wifiRepository.setWifiActivity(activity) + yield() + + assertThat(latest).isTrue() + + job.cancel() + } + + @Test + fun activityContainer_hasActivityOutTrue_outputsTrue() = runBlocking(IMMEDIATE) { + whenever(constants.shouldShowActivityConfig).thenReturn(true) + createAndSetViewModel() + wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) + + var latest: Boolean? = null + val job = underTest + .isActivityContainerVisible + .onEach { latest = it } + .launchIn(this) + + val activity = WifiActivityModel(hasActivityIn = false, hasActivityOut = true) + wifiRepository.setWifiActivity(activity) + yield() + + assertThat(latest).isTrue() + + job.cancel() + } + + @Test + fun activityContainer_inAndOutTrue_outputsTrue() = runBlocking(IMMEDIATE) { + whenever(constants.shouldShowActivityConfig).thenReturn(true) + createAndSetViewModel() + wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) + + var latest: Boolean? = null + val job = underTest + .isActivityContainerVisible + .onEach { latest = it } + .launchIn(this) + + val activity = WifiActivityModel(hasActivityIn = true, hasActivityOut = true) + wifiRepository.setWifiActivity(activity) + yield() + + assertThat(latest).isTrue() + + job.cancel() + } + + @Test + fun activityContainer_inAndOutFalse_outputsFalse() = runBlocking(IMMEDIATE) { + whenever(constants.shouldShowActivityConfig).thenReturn(true) + createAndSetViewModel() + wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) + + var latest: Boolean? = null + val job = underTest + .isActivityContainerVisible + .onEach { latest = it } + .launchIn(this) + + val activity = WifiActivityModel(hasActivityIn = false, hasActivityOut = false) + wifiRepository.setWifiActivity(activity) + yield() + + assertThat(latest).isFalse() + + job.cancel() + } + + private fun createAndSetViewModel() { + // [WifiViewModel] creates its flows as soon as it's instantiated, and some of those flow + // creations rely on certain config values that we mock out in individual tests. This method + // allows tests to create the view model only after those configs are correctly set up. + underTest = WifiViewModel( + constants, + context, + logger, + interactor, + statusBarPipelineFlags, + ) + } + private fun ContentDescription.getAsString(): String? { return when (this) { is ContentDescription.Loaded -> this.description From e41d5cc7632b9aa615448e2f6218a4d00a81cbc1 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Tue, 20 Sep 2022 17:57:16 +0000 Subject: [PATCH 3/8] [SB Refactor] Turn the wifi ViewModel into an @SysUISingleton and instead create separate view models per location. This CL eliminates the many duplicate logs by: 1) Making WifiViewModel a Singleton, so we only ever have one of each flow. 2) Adding 1 ViewModel class per location, which just references the singleton view model flows. 3) Making each flow inside WifiViewModel a StateFlow, so that its logic (including its logging logic) isn't duplicated each time we re-use one of the flows. Bug: 238425913 Test: manual: Verified wifi icon is tinted different colors in each of the 3 locations Test: manual: Verify wifi icon still updates Test: manual: Verify we don't get duplicate activity logs Test: statusbar.pipeline tests Change-Id: I6ab0245a83858875c4e63baf9bb6a8c482d1fe55 --- .../qs/QuickStatusBarHeaderController.java | 3 +- .../shade/LargeScreenShadeHeaderController.kt | 3 +- .../KeyguardStatusBarViewController.java | 4 +- .../phone/StatusBarIconController.java | 46 ++-- .../statusbar/phone/StatusBarLocation.kt | 27 +++ .../fragment/CollapsedStatusBarFragment.java | 4 +- .../pipeline/wifi/ui/binder/WifiViewBinder.kt | 24 ++- .../wifi/ui/view/ModernStatusBarWifiView.kt | 8 +- .../wifi/ui/viewmodel/HomeWifiViewModel.kt | 42 ++++ .../ui/viewmodel/KeyguardWifiViewModel.kt | 39 ++++ .../viewmodel/LocationBasedWifiViewModel.kt | 67 ++++++ .../wifi/ui/viewmodel/QsWifiViewModel.kt | 39 ++++ .../wifi/ui/viewmodel/WifiViewModel.kt | 166 +++++++++----- .../qs/QuickStatusBarHeaderControllerTest.kt | 2 +- ...ScreenShadeHeaderControllerCombinedTest.kt | 2 +- .../LargeScreenShadeHeaderControllerTest.kt | 2 +- .../KeyguardStatusBarViewControllerTest.java | 2 +- .../phone/StatusBarIconControllerTest.java | 14 +- .../CollapsedStatusBarFragmentTest.java | 2 +- .../ui/view/ModernStatusBarWifiViewTest.kt | 3 +- .../wifi/ui/viewmodel/WifiViewModelTest.kt | 203 ++++++++++++++---- 21 files changed, 565 insertions(+), 137 deletions(-) create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarLocation.kt create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/HomeWifiViewModel.kt create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/KeyguardWifiViewModel.kt create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/LocationBasedWifiViewModel.kt create mode 100644 packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/QsWifiViewModel.kt diff --git a/packages/SystemUI/src/com/android/systemui/qs/QuickStatusBarHeaderController.java b/packages/SystemUI/src/com/android/systemui/qs/QuickStatusBarHeaderController.java index b5859616f3927..ccaab1adaf266 100644 --- a/packages/SystemUI/src/com/android/systemui/qs/QuickStatusBarHeaderController.java +++ b/packages/SystemUI/src/com/android/systemui/qs/QuickStatusBarHeaderController.java @@ -30,6 +30,7 @@ import com.android.systemui.qs.carrier.QSCarrierGroupController; import com.android.systemui.qs.dagger.QSScope; import com.android.systemui.statusbar.phone.StatusBarContentInsetsProvider; import com.android.systemui.statusbar.phone.StatusBarIconController; +import com.android.systemui.statusbar.phone.StatusBarLocation; import com.android.systemui.statusbar.phone.StatusIconContainer; import com.android.systemui.statusbar.policy.Clock; import com.android.systemui.statusbar.policy.VariableDateViewController; @@ -104,7 +105,7 @@ class QuickStatusBarHeaderController extends ViewController { diff --git a/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt b/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt index fe40d4cbe23a3..d3ed47407b9d2 100644 --- a/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt +++ b/packages/SystemUI/src/com/android/systemui/shade/LargeScreenShadeHeaderController.kt @@ -48,6 +48,7 @@ import com.android.systemui.shade.LargeScreenShadeHeaderController.Companion.QQS import com.android.systemui.shade.LargeScreenShadeHeaderController.Companion.QS_HEADER_CONSTRAINT import com.android.systemui.statusbar.phone.StatusBarContentInsetsProvider import com.android.systemui.statusbar.phone.StatusBarIconController +import com.android.systemui.statusbar.phone.StatusBarLocation import com.android.systemui.statusbar.phone.StatusIconContainer import com.android.systemui.statusbar.phone.dagger.CentralSurfacesComponent.CentralSurfacesScope import com.android.systemui.statusbar.phone.dagger.StatusBarViewModule.LARGE_SCREEN_BATTERY_CONTROLLER @@ -261,7 +262,7 @@ class LargeScreenShadeHeaderController @Inject constructor( batteryMeterViewController.ignoreTunerUpdates() batteryIcon.setPercentShowMode(BatteryMeterView.MODE_ESTIMATE) - iconManager = tintedIconManagerFactory.create(iconContainer) + iconManager = tintedIconManagerFactory.create(iconContainer, StatusBarLocation.QS) iconManager.setTint( Utils.getColorAttrDefaultColor(header.context, android.R.attr.textColorPrimary) ) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/KeyguardStatusBarViewController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/KeyguardStatusBarViewController.java index ce2c9c2446962..0026b71a53049 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/KeyguardStatusBarViewController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/KeyguardStatusBarViewController.java @@ -352,8 +352,8 @@ public class KeyguardStatusBarViewController extends ViewController wifiViewModelProvider, + WifiViewModel wifiViewModel, MobileContextProvider mobileContextProvider, DarkIconDispatcher darkIconDispatcher) { super(linearLayout, + location, statusBarPipelineFlags, - wifiViewModelProvider, + wifiViewModel, mobileContextProvider); mIconHPadding = mContext.getResources().getDimensionPixelSize( R.dimen.status_bar_icon_padding); @@ -204,27 +205,28 @@ public interface StatusBarIconController { @SysUISingleton public static class Factory { private final StatusBarPipelineFlags mStatusBarPipelineFlags; - private final Provider mWifiViewModelProvider; + private final WifiViewModel mWifiViewModel; private final MobileContextProvider mMobileContextProvider; private final DarkIconDispatcher mDarkIconDispatcher; @Inject public Factory( StatusBarPipelineFlags statusBarPipelineFlags, - Provider wifiViewModelProvider, + WifiViewModel wifiViewModel, MobileContextProvider mobileContextProvider, DarkIconDispatcher darkIconDispatcher) { mStatusBarPipelineFlags = statusBarPipelineFlags; - mWifiViewModelProvider = wifiViewModelProvider; + mWifiViewModel = wifiViewModel; mMobileContextProvider = mobileContextProvider; mDarkIconDispatcher = darkIconDispatcher; } - public DarkIconManager create(LinearLayout group) { + public DarkIconManager create(LinearLayout group, StatusBarLocation location) { return new DarkIconManager( group, + location, mStatusBarPipelineFlags, - mWifiViewModelProvider, + mWifiViewModel, mMobileContextProvider, mDarkIconDispatcher); } @@ -239,12 +241,14 @@ public interface StatusBarIconController { public TintedIconManager( ViewGroup group, + StatusBarLocation location, StatusBarPipelineFlags statusBarPipelineFlags, - Provider wifiViewModelProvider, + WifiViewModel wifiViewModel, MobileContextProvider mobileContextProvider) { super(group, + location, statusBarPipelineFlags, - wifiViewModelProvider, + wifiViewModel, mobileContextProvider); } @@ -278,24 +282,25 @@ public interface StatusBarIconController { @SysUISingleton public static class Factory { private final StatusBarPipelineFlags mStatusBarPipelineFlags; - private final Provider mWifiViewModelProvider; + private final WifiViewModel mWifiViewModel; private final MobileContextProvider mMobileContextProvider; @Inject public Factory( StatusBarPipelineFlags statusBarPipelineFlags, - Provider wifiViewModelProvider, + WifiViewModel wifiViewModel, MobileContextProvider mobileContextProvider) { mStatusBarPipelineFlags = statusBarPipelineFlags; - mWifiViewModelProvider = wifiViewModelProvider; + mWifiViewModel = wifiViewModel; mMobileContextProvider = mobileContextProvider; } - public TintedIconManager create(ViewGroup group) { + public TintedIconManager create(ViewGroup group, StatusBarLocation location) { return new TintedIconManager( group, + location, mStatusBarPipelineFlags, - mWifiViewModelProvider, + mWifiViewModel, mMobileContextProvider); } } @@ -306,8 +311,9 @@ public interface StatusBarIconController { */ class IconManager implements DemoModeCommandReceiver { protected final ViewGroup mGroup; + private final StatusBarLocation mLocation; private final StatusBarPipelineFlags mStatusBarPipelineFlags; - private final Provider mWifiViewModelProvider; + private final WifiViewModel mWifiViewModel; private final MobileContextProvider mMobileContextProvider; protected final Context mContext; protected final int mIconSize; @@ -324,12 +330,14 @@ public interface StatusBarIconController { public IconManager( ViewGroup group, + StatusBarLocation location, StatusBarPipelineFlags statusBarPipelineFlags, - Provider wifiViewModelProvider, + WifiViewModel wifiViewModel, MobileContextProvider mobileContextProvider) { mGroup = group; + mLocation = location; mStatusBarPipelineFlags = statusBarPipelineFlags; - mWifiViewModelProvider = wifiViewModelProvider; + mWifiViewModel = wifiViewModel; mMobileContextProvider = mobileContextProvider; mContext = group.getContext(); mIconSize = mContext.getResources().getDimensionPixelSize( @@ -446,7 +454,7 @@ public interface StatusBarIconController { private ModernStatusBarWifiView onCreateModernStatusBarWifiView(String slot) { return ModernStatusBarWifiView.constructAndBind( - mContext, slot, mWifiViewModelProvider.get()); + mContext, slot, mWifiViewModel, mLocation); } private StatusBarMobileView onCreateStatusBarMobileView(int subId, String slot) { diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarLocation.kt b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarLocation.kt new file mode 100644 index 0000000000000..5ace22695ec37 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/StatusBarLocation.kt @@ -0,0 +1,27 @@ +/* + * 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.phone + +/** An enumeration of the different locations that host a status bar. */ +enum class StatusBarLocation { + /** Home screen or in-app. */ + HOME, + /** Keyguard (aka lockscreen). */ + KEYGUARD, + /** Quick settings (inside the shade). */ + QS, +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragment.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragment.java index ce04fb5999635..e1215ee952380 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragment.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragment.java @@ -68,6 +68,7 @@ import com.android.systemui.statusbar.phone.PhoneStatusBarView; import com.android.systemui.statusbar.phone.StatusBarHideIconsForBouncerManager; import com.android.systemui.statusbar.phone.StatusBarIconController; import com.android.systemui.statusbar.phone.StatusBarIconController.DarkIconManager; +import com.android.systemui.statusbar.phone.StatusBarLocation; import com.android.systemui.statusbar.phone.StatusBarLocationPublisher; import com.android.systemui.statusbar.phone.fragment.dagger.StatusBarFragmentComponent; import com.android.systemui.statusbar.phone.fragment.dagger.StatusBarFragmentComponent.Startable; @@ -250,7 +251,8 @@ public class CollapsedStatusBarFragment extends Fragment implements CommandQueue mStatusBar.restoreHierarchyState( savedInstanceState.getSparseParcelableArray(EXTRA_PANEL_STATE)); } - mDarkIconManager = mDarkIconManagerFactory.create(view.findViewById(R.id.statusIcons)); + mDarkIconManager = mDarkIconManagerFactory.create( + view.findViewById(R.id.statusIcons), StatusBarLocation.HOME); mDarkIconManager.setShouldLog(true); updateBlockedIcons(); mStatusBarIconController.addIconGroup(mDarkIconManager); diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt index 26667ab6d413c..c3a9b90c5a627 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt @@ -26,6 +26,8 @@ import androidx.lifecycle.repeatOnLifecycle import com.android.systemui.R import com.android.systemui.common.ui.binder.IconViewBinder import com.android.systemui.lifecycle.repeatWhenAttached +import com.android.systemui.statusbar.phone.StatusBarLocation +import com.android.systemui.statusbar.pipeline.wifi.ui.viewmodel.LocationBasedWifiViewModel import com.android.systemui.statusbar.pipeline.wifi.ui.viewmodel.WifiViewModel import kotlinx.coroutines.InternalCoroutinesApi import kotlinx.coroutines.flow.collect @@ -41,11 +43,29 @@ import kotlinx.coroutines.launch */ @OptIn(InternalCoroutinesApi::class) object WifiViewBinder { - /** Binds the view to the view-model, continuing to update the former based on the latter. */ + + /** + * Binds the view to the appropriate view-model based on the given location. The view will + * continue to be updated following updates from the view-model. + */ @JvmStatic fun bind( view: ViewGroup, - viewModel: WifiViewModel, + wifiViewModel: WifiViewModel, + location: StatusBarLocation, + ) { + when (location) { + StatusBarLocation.HOME -> bind(view, wifiViewModel.home) + StatusBarLocation.KEYGUARD -> bind(view, wifiViewModel.keyguard) + StatusBarLocation.QS -> bind(view, wifiViewModel.qs) + } + } + + /** Binds the view to the view-model, continuing to update the former based on the latter. */ + @JvmStatic + private fun bind( + view: ViewGroup, + viewModel: LocationBasedWifiViewModel, ) { val iconView = view.requireViewById(R.id.wifi_signal) val activityInView = view.requireViewById(R.id.wifi_in) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiView.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiView.kt index c14a897fffab3..f225b65179ed1 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiView.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiView.kt @@ -23,6 +23,7 @@ import android.view.LayoutInflater import com.android.systemui.R import com.android.systemui.statusbar.BaseStatusBarWifiView import com.android.systemui.statusbar.StatusBarIconView.STATE_ICON +import com.android.systemui.statusbar.phone.StatusBarLocation import com.android.systemui.statusbar.pipeline.wifi.ui.binder.WifiViewBinder import com.android.systemui.statusbar.pipeline.wifi.ui.viewmodel.WifiViewModel @@ -72,21 +73,22 @@ class ModernStatusBarWifiView( companion object { /** - * Inflates a new instance of [ModernStatusBarWifiView], binds it to [viewModel], and + * Inflates a new instance of [ModernStatusBarWifiView], binds it to a view model, and * returns it. */ @JvmStatic fun constructAndBind( context: Context, slot: String, - viewModel: WifiViewModel, + wifiViewModel: WifiViewModel, + location: StatusBarLocation, ): ModernStatusBarWifiView { return ( LayoutInflater.from(context).inflate(R.layout.new_status_bar_wifi_group, null) as ModernStatusBarWifiView ).also { it.setSlot(slot) - WifiViewBinder.bind(it, viewModel) + WifiViewBinder.bind(it, wifiViewModel, location) } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/HomeWifiViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/HomeWifiViewModel.kt new file mode 100644 index 0000000000000..0847e6214337f --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/HomeWifiViewModel.kt @@ -0,0 +1,42 @@ +/* + * 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.wifi.ui.viewmodel + +import android.graphics.Color +import com.android.systemui.common.shared.model.Icon +import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags +import kotlinx.coroutines.flow.Flow + +/** + * A view model for the wifi icon shown on the "home" page (aka, when the device is unlocked and not + * showing the shade, so the user is on the home-screen, or in an app). + */ +class HomeWifiViewModel( + statusBarPipelineFlags: StatusBarPipelineFlags, + wifiIcon: Flow, + isActivityInViewVisible: Flow, + isActivityOutViewVisible: Flow, + isActivityContainerVisible: Flow, +) : + LocationBasedWifiViewModel( + statusBarPipelineFlags, + debugTint = Color.CYAN, + wifiIcon, + isActivityInViewVisible, + isActivityOutViewVisible, + isActivityContainerVisible, + ) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/KeyguardWifiViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/KeyguardWifiViewModel.kt new file mode 100644 index 0000000000000..3f7c8e13d7f72 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/KeyguardWifiViewModel.kt @@ -0,0 +1,39 @@ +/* + * 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.wifi.ui.viewmodel + +import android.graphics.Color +import com.android.systemui.common.shared.model.Icon +import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags +import kotlinx.coroutines.flow.Flow + +/** A view model for the wifi icon shown on keyguard (lockscreen). */ +class KeyguardWifiViewModel( + statusBarPipelineFlags: StatusBarPipelineFlags, + wifiIcon: Flow, + isActivityInViewVisible: Flow, + isActivityOutViewVisible: Flow, + isActivityContainerVisible: Flow, +) : + LocationBasedWifiViewModel( + statusBarPipelineFlags, + debugTint = Color.MAGENTA, + wifiIcon, + isActivityInViewVisible, + isActivityOutViewVisible, + isActivityContainerVisible, + ) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/LocationBasedWifiViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/LocationBasedWifiViewModel.kt new file mode 100644 index 0000000000000..d34ba88cde1a8 --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/LocationBasedWifiViewModel.kt @@ -0,0 +1,67 @@ +/* + * 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.wifi.ui.viewmodel + +import android.graphics.Color +import com.android.systemui.common.shared.model.Icon +import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flowOf + +/** + * A view model for a wifi icon in a specific location. This allows us to control parameters that + * are location-specific (for example, different tints of the icon in different locations). + * + * Must be subclassed for each distinct location. + */ +abstract class LocationBasedWifiViewModel( + statusBarPipelineFlags: StatusBarPipelineFlags, + debugTint: Int, + + /** The wifi icon that should be displayed. Null if we shouldn't display any icon. */ + val wifiIcon: Flow, + + /** True if the activity in view should be visible. */ + val isActivityInViewVisible: Flow, + + /** True if the activity out view should be visible. */ + val isActivityOutViewVisible: Flow, + + /** True if the activity container view should be visible. */ + val isActivityContainerVisible: Flow, +) { + /** The color that should be used to tint the icon. */ + val tint: Flow = + flowOf( + if (statusBarPipelineFlags.useNewPipelineDebugColoring()) { + debugTint + } else { + DEFAULT_TINT + } + ) + + companion object { + /** + * A default icon tint. + * + * TODO(b/238425913): The tint is actually controlled by + * [com.android.systemui.statusbar.phone.StatusBarIconController.TintedIconManager]. We + * should use that logic instead of white as a default. + */ + private const val DEFAULT_TINT = Color.WHITE + } +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/QsWifiViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/QsWifiViewModel.kt new file mode 100644 index 0000000000000..8f6c26a6620fa --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/QsWifiViewModel.kt @@ -0,0 +1,39 @@ +/* + * 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.wifi.ui.viewmodel + +import android.graphics.Color +import com.android.systemui.common.shared.model.Icon +import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags +import kotlinx.coroutines.flow.Flow + +/** A view model for the wifi icon shown in quick settings (when the shade is pulled down). */ +class QsWifiViewModel( + statusBarPipelineFlags: StatusBarPipelineFlags, + wifiIcon: Flow, + isActivityInViewVisible: Flow, + isActivityOutViewVisible: Flow, + isActivityContainerVisible: Flow, +) : + LocationBasedWifiViewModel( + statusBarPipelineFlags, + debugTint = Color.GREEN, + wifiIcon, + isActivityInViewVisible, + isActivityOutViewVisible, + isActivityContainerVisible, + ) 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 8197e89cb93ef..dae701436be29 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 @@ -17,7 +17,6 @@ package com.android.systemui.statusbar.pipeline.wifi.ui.viewmodel import android.content.Context -import android.graphics.Color import androidx.annotation.DrawableRes import androidx.annotation.StringRes import androidx.annotation.VisibleForTesting @@ -26,6 +25,8 @@ import com.android.settingslib.AccessibilityContentDescriptions.WIFI_NO_CONNECTI import com.android.systemui.R import com.android.systemui.common.shared.model.ContentDescription import com.android.systemui.common.shared.model.Icon +import com.android.systemui.dagger.SysUISingleton +import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.statusbar.connectivity.WifiIcons.WIFI_FULL_ICONS import com.android.systemui.statusbar.connectivity.WifiIcons.WIFI_NO_INTERNET_ICONS import com.android.systemui.statusbar.connectivity.WifiIcons.WIFI_NO_NETWORK @@ -37,68 +38,90 @@ import com.android.systemui.statusbar.pipeline.wifi.domain.interactor.WifiIntera import com.android.systemui.statusbar.pipeline.wifi.shared.WifiConstants import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiActivityModel import javax.inject.Inject +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.distinctUntilChanged -import kotlinx.coroutines.flow.emptyFlow import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.stateIn /** * Models the UI state for the status bar wifi icon. + * + * This class exposes three view models, one per status bar location: + * - [home] + * - [keyguard] + * - [qs] + * In order to get the UI state for the wifi icon, you must use one of those view models (whichever + * is correct for your location). + * + * Internally, this class maintains the current state of the wifi icon and notifies those three + * view models of any changes. */ -class WifiViewModel @Inject constructor( +@SysUISingleton +class WifiViewModel +@Inject +constructor( constants: WifiConstants, private val context: Context, logger: ConnectivityPipelineLogger, interactor: WifiInteractor, + @Application private val scope: CoroutineScope, statusBarPipelineFlags: StatusBarPipelineFlags, ) { - /** - * The drawable resource ID to use for the wifi icon. Null if we shouldn't display any icon. - */ + /** The drawable resource ID to use for the wifi icon. Null if we shouldn't display any icon. */ @DrawableRes - private val iconResId: Flow = interactor.wifiNetwork.map { - when (it) { - is WifiNetworkModel.CarrierMerged -> null - is WifiNetworkModel.Inactive -> WIFI_NO_NETWORK - is WifiNetworkModel.Active -> - when { - it.level == null -> null - it.isValidated -> WIFI_FULL_ICONS[it.level] - else -> WIFI_NO_INTERNET_ICONS[it.level] + private val iconResId: Flow = + interactor.wifiNetwork + .map { + when (it) { + is WifiNetworkModel.CarrierMerged -> null + is WifiNetworkModel.Inactive -> WIFI_NO_NETWORK + is WifiNetworkModel.Active -> + when { + it.level == null -> null + it.isValidated -> WIFI_FULL_ICONS[it.level] + else -> WIFI_NO_INTERNET_ICONS[it.level] + } } - } - } + } + .stateIn(scope, started = SharingStarted.WhileSubscribed(), initialValue = null) /** The content description for the wifi icon. */ - private val contentDescription: Flow = interactor.wifiNetwork.map { - when (it) { - is WifiNetworkModel.CarrierMerged -> null - is WifiNetworkModel.Inactive -> - ContentDescription.Loaded( - "${context.getString(WIFI_NO_CONNECTION)},${context.getString(NO_INTERNET)}" - ) - is WifiNetworkModel.Active -> - when (it.level) { - null -> null - else -> { - val levelDesc = context.getString(WIFI_CONNECTION_STRENGTH[it.level]) - when { - it.isValidated -> ContentDescription.Loaded(levelDesc) - else -> ContentDescription.Loaded( - "$levelDesc,${context.getString(NO_INTERNET)}" - ) + private val contentDescription: Flow = + interactor.wifiNetwork + .map { + when (it) { + is WifiNetworkModel.CarrierMerged -> null + is WifiNetworkModel.Inactive -> + ContentDescription.Loaded( + "${context.getString(WIFI_NO_CONNECTION)}," + + context.getString(NO_INTERNET) + ) + is WifiNetworkModel.Active -> + when (it.level) { + null -> null + else -> { + val levelDesc = + context.getString(WIFI_CONNECTION_STRENGTH[it.level]) + when { + it.isValidated -> ContentDescription.Loaded(levelDesc) + else -> + ContentDescription.Loaded( + "$levelDesc,${context.getString(NO_INTERNET)}" + ) + } + } } - } } - } - } + } + .stateIn(scope, started = SharingStarted.WhileSubscribed(), initialValue = null) - /** - * The wifi icon that should be displayed. Null if we shouldn't display any icon. - */ - val wifiIcon: Flow = combine( + /** The wifi icon that should be displayed. Null if we shouldn't display any icon. */ + private val wifiIcon: Flow = + combine( interactor.isForceHidden, iconResId, contentDescription, @@ -110,6 +133,7 @@ class WifiViewModel @Inject constructor( else -> Icon.Resource(iconResId, contentDescription) } } + .stateIn(scope, started = SharingStarted.WhileSubscribed(), initialValue = null) /** The wifi activity status. Null if we shouldn't display the activity status. */ private val activity: Flow = @@ -125,29 +149,53 @@ class WifiViewModel @Inject constructor( } .distinctUntilChanged() .logOutputChange(logger, "activity") + .stateIn(scope, started = SharingStarted.WhileSubscribed(), initialValue = null) - /** True if the activity in view should be visible. */ - val isActivityInViewVisible: Flow = activity.map { it?.hasActivityIn == true } + private val isActivityInViewVisible: Flow = + activity + .map { it?.hasActivityIn == true } + .stateIn(scope, started = SharingStarted.WhileSubscribed(), initialValue = false) - /** True if the activity out view should be visible. */ - val isActivityOutViewVisible: Flow = activity.map { it?.hasActivityOut == true } + private val isActivityOutViewVisible: Flow = + activity + .map { it?.hasActivityOut == true } + .stateIn(scope, started = SharingStarted.WhileSubscribed(), initialValue = false) - /** True if the activity container view should be visible. */ - val isActivityContainerVisible: Flow = - combine(isActivityInViewVisible, isActivityOutViewVisible) { activityIn, activityOut -> - activityIn || activityOut - } + private val isActivityContainerVisible: Flow = + combine(isActivityInViewVisible, isActivityOutViewVisible) { activityIn, activityOut -> + activityIn || activityOut + } + .stateIn(scope, started = SharingStarted.WhileSubscribed(), initialValue = false) - // TODO(b/238425913): Update this class to use state flows instead. Right now, we have a ton of - // duplicate activity logs because the cold flows are getting duplicated for the three - // activityVisible flows. + /** A view model for the status bar on the home screen. */ + val home: HomeWifiViewModel = + HomeWifiViewModel( + statusBarPipelineFlags, + wifiIcon, + isActivityInViewVisible, + isActivityOutViewVisible, + isActivityContainerVisible, + ) - /** The tint that should be applied to the icon. */ - val tint: Flow = if (!statusBarPipelineFlags.useNewPipelineDebugColoring()) { - emptyFlow() - } else { - flowOf(Color.CYAN) - } + /** A view model for the status bar on keyguard. */ + val keyguard: KeyguardWifiViewModel = + KeyguardWifiViewModel( + statusBarPipelineFlags, + wifiIcon, + isActivityInViewVisible, + isActivityOutViewVisible, + isActivityContainerVisible, + ) + + /** A view model for the status bar in quick settings. */ + val qs: QsWifiViewModel = + QsWifiViewModel( + statusBarPipelineFlags, + wifiIcon, + isActivityInViewVisible, + isActivityOutViewVisible, + isActivityContainerVisible, + ) companion object { @StringRes diff --git a/packages/SystemUI/tests/src/com/android/systemui/qs/QuickStatusBarHeaderControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/qs/QuickStatusBarHeaderControllerTest.kt index eb907bd924714..39d89bf99af25 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/qs/QuickStatusBarHeaderControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/qs/QuickStatusBarHeaderControllerTest.kt @@ -110,7 +110,7 @@ class QuickStatusBarHeaderControllerTest : SysuiTestCase() { `when`(qsCarrierGroupControllerBuilder.build()).thenReturn(qsCarrierGroupController) `when`(variableDateViewControllerFactory.create(any())) .thenReturn(variableDateViewController) - `when`(iconManagerFactory.create(any())).thenReturn(iconManager) + `when`(iconManagerFactory.create(any(), any())).thenReturn(iconManager) `when`(view.resources).thenReturn(mContext.resources) `when`(view.isAttachedToWindow).thenReturn(true) `when`(view.context).thenReturn(context) diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerCombinedTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerCombinedTest.kt index c4485389d646d..c76d9e7a2b200 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerCombinedTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerCombinedTest.kt @@ -176,7 +176,7 @@ class LargeScreenShadeHeaderControllerCombinedTest : SysuiTestCase() { } whenever(view.visibility).thenAnswer { _ -> viewVisibility } - whenever(iconManagerFactory.create(any())).thenReturn(iconManager) + whenever(iconManagerFactory.create(any(), any())).thenReturn(iconManager) whenever(featureFlags.isEnabled(Flags.COMBINED_QS_HEADERS)).thenReturn(true) whenever(featureFlags.isEnabled(Flags.NEW_HEADER)).thenReturn(true) diff --git a/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerTest.kt index 5ecfc8eb3649f..90ae693db955e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/shade/LargeScreenShadeHeaderControllerTest.kt @@ -97,7 +97,7 @@ class LargeScreenShadeHeaderControllerTest : SysuiTestCase() { whenever(view.visibility).thenAnswer { _ -> viewVisibility } whenever(variableDateViewControllerFactory.create(any())) .thenReturn(variableDateViewController) - whenever(iconManagerFactory.create(any())).thenReturn(iconManager) + whenever(iconManagerFactory.create(any(), any())).thenReturn(iconManager) whenever(featureFlags.isEnabled(Flags.COMBINED_QS_HEADERS)).thenReturn(false) mLargeScreenShadeHeaderController = LargeScreenShadeHeaderController( view, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/KeyguardStatusBarViewControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/KeyguardStatusBarViewControllerTest.java index ba5f5038c1d97..cfaa4707ef763 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/KeyguardStatusBarViewControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/KeyguardStatusBarViewControllerTest.java @@ -135,7 +135,7 @@ public class KeyguardStatusBarViewControllerTest extends SysuiTestCase { MockitoAnnotations.initMocks(this); - when(mIconManagerFactory.create(any())).thenReturn(mIconManager); + when(mIconManagerFactory.create(any(), any())).thenReturn(mIconManager); allowTestableLooperAsMainThread(); TestableLooper.get(this).runWithLooper(() -> { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerTest.java index de7db74495af6..34399b80c9f7e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/StatusBarIconControllerTest.java @@ -51,8 +51,6 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; -import javax.inject.Provider; - @RunWith(AndroidTestingRunner.class) @RunWithLooper @SmallTest @@ -79,8 +77,9 @@ public class StatusBarIconControllerTest extends LeakCheckedTest { LinearLayout layout = new LinearLayout(mContext); TestDarkIconManager manager = new TestDarkIconManager( layout, + StatusBarLocation.HOME, mock(StatusBarPipelineFlags.class), - () -> mock(WifiViewModel.class), + mock(WifiViewModel.class), mMobileContextProvider, mock(DarkIconDispatcher.class)); testCallOnAdd_forManager(manager); @@ -121,13 +120,15 @@ public class StatusBarIconControllerTest extends LeakCheckedTest { TestDarkIconManager( LinearLayout group, + StatusBarLocation location, StatusBarPipelineFlags statusBarPipelineFlags, - Provider wifiViewModelProvider, + WifiViewModel wifiViewModel, MobileContextProvider contextProvider, DarkIconDispatcher darkIconDispatcher) { super(group, + location, statusBarPipelineFlags, - wifiViewModelProvider, + wifiViewModel, contextProvider, darkIconDispatcher); } @@ -165,8 +166,9 @@ public class StatusBarIconControllerTest extends LeakCheckedTest { private static class TestIconManager extends IconManager implements TestableIconManager { TestIconManager(ViewGroup group, MobileContextProvider contextProvider) { super(group, + StatusBarLocation.HOME, mock(StatusBarPipelineFlags.class), - () -> mock(WifiViewModel.class), + mock(WifiViewModel.class), contextProvider); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragmentTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragmentTest.java index 37c8f6285970c..a3c6e95141917 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragmentTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragmentTest.java @@ -431,7 +431,7 @@ public class CollapsedStatusBarFragmentTest extends SysuiBaseFragmentTest { mOperatorNameViewControllerFactory = mock(OperatorNameViewController.Factory.class); when(mOperatorNameViewControllerFactory.create(any())) .thenReturn(mOperatorNameViewController); - when(mIconManagerFactory.create(any())).thenReturn(mIconManager); + when(mIconManagerFactory.create(any(), any())).thenReturn(mIconManager); mSecureSettings = mock(SecureSettings.class); setUpNotificationIconAreaController(); diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt index 3c200a5da4fae..cbb4b7e051761 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt @@ -20,6 +20,7 @@ import android.testing.TestableLooper.RunWithLooper import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.lifecycle.InstantTaskExecutorRule +import com.android.systemui.statusbar.phone.StatusBarLocation import com.android.systemui.util.Assert import com.android.systemui.util.mockito.mock import com.google.common.truth.Truth.assertThat @@ -45,7 +46,7 @@ class ModernStatusBarWifiViewTest : SysuiTestCase() { @Test fun constructAndBind_hasCorrectSlot() { val view = ModernStatusBarWifiView.constructAndBind( - context, "slotName", mock() + context, "slotName", mock(), StatusBarLocation.HOME ) assertThat(view.slot).isEqualTo("slotName") 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 f0ef9d043169c..ca30285338c3b 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 @@ -36,12 +36,15 @@ import com.android.systemui.statusbar.pipeline.wifi.shared.WifiConstants import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiActivityModel import com.android.systemui.statusbar.pipeline.wifi.ui.viewmodel.WifiViewModel.Companion.NO_INTERNET import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.runBlocking import kotlinx.coroutines.yield +import org.junit.After import org.junit.Before import org.junit.Test import org.mockito.Mock @@ -60,6 +63,7 @@ class WifiViewModelTest : SysuiTestCase() { private lateinit var connectivityRepository: FakeConnectivityRepository private lateinit var wifiRepository: FakeWifiRepository private lateinit var interactor: WifiInteractor + private lateinit var scope: CoroutineScope @Before fun setUp() { @@ -67,20 +71,35 @@ class WifiViewModelTest : SysuiTestCase() { connectivityRepository = FakeConnectivityRepository() wifiRepository = FakeWifiRepository() interactor = WifiInteractor(connectivityRepository, wifiRepository) + scope = CoroutineScope(IMMEDIATE) createAndSetViewModel() } + @After + fun tearDown() { + scope.cancel() + } + + // Note on testing: [WifiViewModel] exposes 3 different instances of + // [LocationBasedWifiViewModel]. In practice, these 3 different instances will get the exact + // same data for icon, activity, etc. flows. So, most of these tests will test just one of the + // instances. There are also some tests that verify all 3 instances received the same data. + @Test fun wifiIcon_forceHidden_outputsNull() = runBlocking(IMMEDIATE) { connectivityRepository.setForceHiddenIcons(setOf(ConnectivitySlot.WIFI)) - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(NETWORK_ID, level = 2)) - var latest: Icon? = null + // Start as non-null so we can verify we got the update + var latest: Icon? = Icon.Resource(0, null) val job = underTest + .home .wifiIcon .onEach { latest = it } .launchIn(this) + wifiRepository.setWifiNetwork(WifiNetworkModel.Active(NETWORK_ID, level = 2)) + yield() + assertThat(latest).isNull() job.cancel() @@ -89,14 +108,17 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun wifiIcon_notForceHidden_outputsVisible() = runBlocking(IMMEDIATE) { connectivityRepository.setForceHiddenIcons(setOf()) - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(NETWORK_ID, level = 2)) var latest: Icon? = null val job = underTest + .home .wifiIcon .onEach { latest = it } .launchIn(this) + wifiRepository.setWifiNetwork(WifiNetworkModel.Active(NETWORK_ID, level = 2)) + yield() + assertThat(latest).isInstanceOf(Icon.Resource::class.java) job.cancel() @@ -104,13 +126,15 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun wifiIcon_inactiveNetwork_outputsNoNetworkIcon() = runBlocking(IMMEDIATE) { - wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive) - var latest: Icon? = null val job = underTest - .wifiIcon - .onEach { latest = it } - .launchIn(this) + .home + .wifiIcon + .onEach { latest = it } + .launchIn(this) + + wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive) + yield() assertThat(latest).isInstanceOf(Icon.Resource::class.java) val icon = latest as Icon.Resource @@ -125,14 +149,16 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun wifiIcon_carrierMergedNetwork_outputsNull() = runBlocking(IMMEDIATE) { - wifiRepository.setWifiNetwork(WifiNetworkModel.CarrierMerged) - - var latest: Icon? = null + var latest: Icon? = Icon.Resource(0, null) val job = underTest + .home .wifiIcon .onEach { latest = it } .launchIn(this) + wifiRepository.setWifiNetwork(WifiNetworkModel.CarrierMerged) + yield() + assertThat(latest).isNull() job.cancel() @@ -140,14 +166,16 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun wifiIcon_isActiveNullLevel_outputsNull() = runBlocking(IMMEDIATE) { - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(NETWORK_ID, level = null)) - - var latest: Icon? = null + var latest: Icon? = Icon.Resource(0, null) val job = underTest + .home .wifiIcon .onEach { latest = it } .launchIn(this) + wifiRepository.setWifiNetwork(WifiNetworkModel.Active(NETWORK_ID, level = null)) + yield() + assertThat(latest).isNull() job.cancel() @@ -155,22 +183,23 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun wifiIcon_isActiveAndValidated_level1_outputsFull1Icon() = runBlocking(IMMEDIATE) { - val level = 1 - - wifiRepository.setWifiNetwork( - WifiNetworkModel.Active( - NETWORK_ID, - isValidated = true, - level = level - ) - ) - var latest: Icon? = null val job = underTest + .home .wifiIcon .onEach { latest = it } .launchIn(this) + val level = 1 + wifiRepository.setWifiNetwork( + WifiNetworkModel.Active( + NETWORK_ID, + isValidated = true, + level, + ) + ) + yield() + assertThat(latest).isInstanceOf(Icon.Resource::class.java) val icon = latest as Icon.Resource assertThat(icon.res).isEqualTo(WIFI_FULL_ICONS[level]) @@ -184,22 +213,23 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun wifiIcon_isActiveAndNotValidated_level4_outputsEmpty4Icon() = runBlocking(IMMEDIATE) { - val level = 4 - - wifiRepository.setWifiNetwork( - WifiNetworkModel.Active( - NETWORK_ID, - isValidated = false, - level = level - ) - ) - var latest: Icon? = null val job = underTest + .home .wifiIcon .onEach { latest = it } .launchIn(this) + val level = 4 + wifiRepository.setWifiNetwork( + WifiNetworkModel.Active( + NETWORK_ID, + isValidated = false, + level, + ) + ) + yield() + assertThat(latest).isInstanceOf(Icon.Resource::class.java) val icon = latest as Icon.Resource assertThat(icon.res).isEqualTo(WIFI_NO_INTERNET_ICONS[level]) @@ -211,6 +241,47 @@ class WifiViewModelTest : SysuiTestCase() { job.cancel() } + @Test + fun wifiIcon_allLocationViewModelsReceiveSameData() = runBlocking(IMMEDIATE) { + var latestHome: Icon? = null + val jobHome = underTest + .home + .wifiIcon + .onEach { latestHome = it } + .launchIn(this) + + var latestKeyguard: Icon? = null + val jobKeyguard = underTest + .keyguard + .wifiIcon + .onEach { latestKeyguard = it } + .launchIn(this) + + var latestQs: Icon? = null + val jobQs = underTest + .qs + .wifiIcon + .onEach { latestQs = it } + .launchIn(this) + + wifiRepository.setWifiNetwork( + WifiNetworkModel.Active( + NETWORK_ID, + isValidated = true, + level = 1 + ) + ) + yield() + + assertThat(latestHome).isInstanceOf(Icon.Resource::class.java) + assertThat(latestHome).isEqualTo(latestKeyguard) + assertThat(latestKeyguard).isEqualTo(latestQs) + + jobHome.cancel() + jobKeyguard.cancel() + jobQs.cancel() + } + @Test fun activity_showActivityConfigFalse_outputsFalse() = runBlocking(IMMEDIATE) { whenever(constants.shouldShowActivityConfig).thenReturn(false) @@ -219,18 +290,21 @@ class WifiViewModelTest : SysuiTestCase() { var activityIn: Boolean? = null val activityInJob = underTest - .isActivityInViewVisible - .onEach { activityIn = it } - .launchIn(this) + .home + .isActivityInViewVisible + .onEach { activityIn = it } + .launchIn(this) var activityOut: Boolean? = null val activityOutJob = underTest + .home .isActivityOutViewVisible .onEach { activityOut = it } .launchIn(this) var activityContainer: Boolean? = null val activityContainerJob = underTest + .home .isActivityContainerVisible .onEach { activityContainer = it } .launchIn(this) @@ -253,18 +327,21 @@ class WifiViewModelTest : SysuiTestCase() { var activityIn: Boolean? = null val activityInJob = underTest + .home .isActivityInViewVisible .onEach { activityIn = it } .launchIn(this) var activityOut: Boolean? = null val activityOutJob = underTest + .home .isActivityOutViewVisible .onEach { activityOut = it } .launchIn(this) var activityContainer: Boolean? = null val activityContainerJob = underTest + .home .isActivityContainerVisible .onEach { activityContainer = it } .launchIn(this) @@ -293,18 +370,21 @@ class WifiViewModelTest : SysuiTestCase() { var activityIn: Boolean? = null val activityInJob = underTest + .home .isActivityInViewVisible .onEach { activityIn = it } .launchIn(this) var activityOut: Boolean? = null val activityOutJob = underTest + .home .isActivityOutViewVisible .onEach { activityOut = it } .launchIn(this) var activityContainer: Boolean? = null val activityContainerJob = underTest + .home .isActivityContainerVisible .onEach { activityContainer = it } .launchIn(this) @@ -324,6 +404,46 @@ class WifiViewModelTest : SysuiTestCase() { activityContainerJob.cancel() } + @Test + fun activity_allLocationViewModelsReceiveSameData() = runBlocking(IMMEDIATE) { + whenever(constants.shouldShowActivityConfig).thenReturn(true) + createAndSetViewModel() + wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK) + + var latestHome: Boolean? = null + val jobHome = underTest + .home + .isActivityInViewVisible + .onEach { latestHome = it } + .launchIn(this) + + var latestKeyguard: Boolean? = null + val jobKeyguard = underTest + .keyguard + .isActivityInViewVisible + .onEach { latestKeyguard = it } + .launchIn(this) + + var latestQs: Boolean? = null + val jobQs = underTest + .qs + .isActivityInViewVisible + .onEach { latestQs = it } + .launchIn(this) + + val activity = WifiActivityModel(hasActivityIn = true, hasActivityOut = true) + wifiRepository.setWifiActivity(activity) + yield() + + assertThat(latestHome).isTrue() + assertThat(latestKeyguard).isTrue() + assertThat(latestQs).isTrue() + + jobHome.cancel() + jobKeyguard.cancel() + jobQs.cancel() + } + @Test fun activityIn_hasActivityInTrue_outputsTrue() = runBlocking(IMMEDIATE) { whenever(constants.shouldShowActivityConfig).thenReturn(true) @@ -332,6 +452,7 @@ class WifiViewModelTest : SysuiTestCase() { var latest: Boolean? = null val job = underTest + .home .isActivityInViewVisible .onEach { latest = it } .launchIn(this) @@ -353,6 +474,7 @@ class WifiViewModelTest : SysuiTestCase() { var latest: Boolean? = null val job = underTest + .home .isActivityInViewVisible .onEach { latest = it } .launchIn(this) @@ -374,6 +496,7 @@ class WifiViewModelTest : SysuiTestCase() { var latest: Boolean? = null val job = underTest + .home .isActivityOutViewVisible .onEach { latest = it } .launchIn(this) @@ -395,6 +518,7 @@ class WifiViewModelTest : SysuiTestCase() { var latest: Boolean? = null val job = underTest + .home .isActivityOutViewVisible .onEach { latest = it } .launchIn(this) @@ -416,6 +540,7 @@ class WifiViewModelTest : SysuiTestCase() { var latest: Boolean? = null val job = underTest + .home .isActivityContainerVisible .onEach { latest = it } .launchIn(this) @@ -437,6 +562,7 @@ class WifiViewModelTest : SysuiTestCase() { var latest: Boolean? = null val job = underTest + .home .isActivityContainerVisible .onEach { latest = it } .launchIn(this) @@ -458,6 +584,7 @@ class WifiViewModelTest : SysuiTestCase() { var latest: Boolean? = null val job = underTest + .home .isActivityContainerVisible .onEach { latest = it } .launchIn(this) @@ -479,6 +606,7 @@ class WifiViewModelTest : SysuiTestCase() { var latest: Boolean? = null val job = underTest + .home .isActivityContainerVisible .onEach { latest = it } .launchIn(this) @@ -501,6 +629,7 @@ class WifiViewModelTest : SysuiTestCase() { context, logger, interactor, + scope, statusBarPipelineFlags, ) } From 40ddf4b82ba2ab88c1c134fb601e859e89928b7f Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Wed, 21 Sep 2022 20:51:38 +0000 Subject: [PATCH 4/8] [SB Refactor] Add tracking for wifi enabled state and pipe it through to the UI. Bug: 238425913 Test: manual: Turn on airplane mode and verify wifi icon disappears (airplane mode => wifiEnabled = false) Test: manual: Turn off airplane mode and verify icon reappears Test: statusbar.pipeline tests Change-Id: Idee192ffc6a26a180f2ff04213ffd3fd8a2f0ee4 --- .../wifi/data/repository/WifiRepository.kt | 42 +++++++++++ .../wifi/domain/interactor/WifiInteractor.kt | 3 + .../wifi/ui/viewmodel/WifiViewModel.kt | 6 +- .../data/repository/FakeWifiRepository.kt | 7 ++ .../data/repository/WifiRepositoryImplTest.kt | 72 +++++++++++++++++++ .../domain/interactor/WifiInteractorTest.kt | 23 ++++++ .../wifi/ui/viewmodel/WifiViewModelTest.kt | 21 ++++++ 7 files changed, 172 insertions(+), 2 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt index 6f20cbc004e1a..1400032ea0d31 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt @@ -36,6 +36,7 @@ import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.SB_LOGGING_TAG +import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logOutputChange import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiActivityModel import java.util.concurrent.Executor @@ -43,13 +44,21 @@ import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.channels.awaitClose +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.mapLatest import kotlinx.coroutines.flow.stateIn /** Provides data related to the wifi state. */ interface WifiRepository { + /** Observable for the current wifi enabled status. */ + val isWifiEnabled: StateFlow + /** Observable for the current wifi network. */ val wifiNetwork: StateFlow @@ -68,6 +77,34 @@ class WifiRepositoryImpl @Inject constructor( @Application scope: CoroutineScope, wifiManager: WifiManager?, ) : WifiRepository { + + /** + * A flow that emits [Unit] whenever the wifi state may have changed. + * + * Because [WifiManager] doesn't expose a wifi state change listener, we do it internally by + * emitting to this flow whenever we think the state may have changed. + * + * TODO(b/238425913): We also need to emit to this flow whenever the WIFI_STATE_CHANGED_ACTION + * intent is triggered. + */ + private val _wifiStateChangeEvents: MutableSharedFlow = + MutableSharedFlow(extraBufferCapacity = 1) + + override val isWifiEnabled: StateFlow = + if (wifiManager == null) { + MutableStateFlow(false).asStateFlow() + } else { + _wifiStateChangeEvents + .mapLatest { wifiManager.isWifiEnabled } + .distinctUntilChanged() + .logOutputChange(logger, "enabled") + .stateIn( + scope = scope, + started = SharingStarted.WhileSubscribed(), + initialValue = wifiManager.isWifiEnabled + ) + } + override val wifiNetwork: StateFlow = conflatedCallbackFlow { var currentWifi: WifiNetworkModel = WIFI_NETWORK_DEFAULT @@ -78,6 +115,8 @@ class WifiRepositoryImpl @Inject constructor( ) { logger.logOnCapabilitiesChanged(network, networkCapabilities) + _wifiStateChangeEvents.tryEmit(Unit) + val wifiInfo = networkCapabilitiesToWifiInfo(networkCapabilities) if (wifiInfo?.isPrimary == true) { val wifiNetworkModel = createWifiNetworkModel( @@ -98,6 +137,9 @@ class WifiRepositoryImpl @Inject constructor( override fun onLost(network: Network) { logger.logOnLost(network) + + _wifiStateChangeEvents.tryEmit(Unit) + val wifi = currentWifi if (wifi is WifiNetworkModel.Active && wifi.networkId == network.getNetId()) { val newNetworkModel = WifiNetworkModel.Inactive diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractor.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractor.kt index ce6003f0abd9b..04b17ed2924af 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractor.kt @@ -56,6 +56,9 @@ class WifiInteractor @Inject constructor( } } + /** Our current enabled status. */ + val isEnabled: Flow = wifiRepository.isWifiEnabled + /** Our current wifi network. See [WifiNetworkModel]. */ val wifiNetwork: Flow = wifiRepository.wifiNetwork 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 dae701436be29..13fd922720f66 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 @@ -122,12 +122,14 @@ constructor( /** The wifi icon that should be displayed. Null if we shouldn't display any icon. */ private val wifiIcon: Flow = combine( + interactor.isEnabled, interactor.isForceHidden, iconResId, contentDescription, - ) { isForceHidden, iconResId, contentDescription -> + ) { isEnabled, isForceHidden, iconResId, contentDescription -> when { - isForceHidden || + !isEnabled || + isForceHidden || iconResId == null || iconResId <= 0 -> null else -> Icon.Resource(iconResId, contentDescription) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt index cd0f27a25b7e2..f751afc195b2d 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/FakeWifiRepository.kt @@ -24,6 +24,9 @@ import kotlinx.coroutines.flow.StateFlow /** Fake implementation of [WifiRepository] exposing set methods for all the flows. */ class FakeWifiRepository : WifiRepository { + private val _isWifiEnabled: MutableStateFlow = MutableStateFlow(false) + override val isWifiEnabled: StateFlow = _isWifiEnabled + private val _wifiNetwork: MutableStateFlow = MutableStateFlow(WifiNetworkModel.Inactive) override val wifiNetwork: StateFlow = _wifiNetwork @@ -31,6 +34,10 @@ class FakeWifiRepository : WifiRepository { private val _wifiActivity = MutableStateFlow(ACTIVITY_DEFAULT) override val wifiActivity: StateFlow = _wifiActivity + fun setIsWifiEnabled(enabled: Boolean) { + _isWifiEnabled.value = enabled + } + fun setWifiNetwork(wifiNetworkModel: WifiNetworkModel) { _wifiNetwork.value = wifiNetworkModel } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepositoryImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepositoryImplTest.kt index 1878ce5b7e17b..c962315adb51b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepositoryImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepositoryImplTest.kt @@ -87,6 +87,78 @@ class WifiRepositoryImplTest : SysuiTestCase() { scope.cancel() } + @Test + fun isWifiEnabled_nullWifiManager_getsFalse() = runBlocking(IMMEDIATE) { + underTest = WifiRepositoryImpl( + connectivityManager, + logger, + executor, + scope, + wifiManager = null, + ) + + assertThat(underTest.isWifiEnabled.value).isFalse() + } + + @Test + fun isWifiEnabled_initiallyGetsWifiManagerValue() = runBlocking(IMMEDIATE) { + whenever(wifiManager.isWifiEnabled).thenReturn(true) + + underTest = WifiRepositoryImpl( + connectivityManager, + logger, + executor, + scope, + wifiManager + ) + + assertThat(underTest.isWifiEnabled.value).isTrue() + } + + @Test + fun isWifiEnabled_networkCapabilitiesChanged_valueUpdated() = runBlocking(IMMEDIATE) { + // We need to call launch on the flows so that they start updating + val networkJob = underTest.wifiNetwork.launchIn(this) + val enabledJob = underTest.isWifiEnabled.launchIn(this) + + whenever(wifiManager.isWifiEnabled).thenReturn(true) + getNetworkCallback().onCapabilitiesChanged( + NETWORK, createWifiNetworkCapabilities(PRIMARY_WIFI_INFO) + ) + + assertThat(underTest.isWifiEnabled.value).isTrue() + + whenever(wifiManager.isWifiEnabled).thenReturn(false) + getNetworkCallback().onCapabilitiesChanged( + NETWORK, createWifiNetworkCapabilities(PRIMARY_WIFI_INFO) + ) + + assertThat(underTest.isWifiEnabled.value).isFalse() + + networkJob.cancel() + enabledJob.cancel() + } + + @Test + fun isWifiEnabled_networkLost_valueUpdated() = runBlocking(IMMEDIATE) { + // We need to call launch on the flows so that they start updating + val networkJob = underTest.wifiNetwork.launchIn(this) + val enabledJob = underTest.isWifiEnabled.launchIn(this) + + whenever(wifiManager.isWifiEnabled).thenReturn(true) + getNetworkCallback().onLost(NETWORK) + + assertThat(underTest.isWifiEnabled.value).isTrue() + + whenever(wifiManager.isWifiEnabled).thenReturn(false) + getNetworkCallback().onLost(NETWORK) + + assertThat(underTest.isWifiEnabled.value).isFalse() + + networkJob.cancel() + enabledJob.cancel() + } + @Test fun wifiNetwork_initiallyGetsDefault() = runBlocking(IMMEDIATE) { var latest: WifiNetworkModel? = null diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractorTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractorTest.kt index 622f20769f0df..39b886af1cb8e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractorTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/domain/interactor/WifiInteractorTest.kt @@ -154,6 +154,29 @@ class WifiInteractorTest : SysuiTestCase() { job.cancel() } + @Test + fun isEnabled_matchesRepoIsEnabled() = runBlocking(IMMEDIATE) { + var latest: Boolean? = null + val job = underTest + .isEnabled + .onEach { latest = it } + .launchIn(this) + + wifiRepository.setIsWifiEnabled(true) + yield() + assertThat(latest).isTrue() + + wifiRepository.setIsWifiEnabled(false) + yield() + assertThat(latest).isFalse() + + wifiRepository.setIsWifiEnabled(true) + yield() + assertThat(latest).isTrue() + + job.cancel() + } + @Test fun wifiNetwork_matchesRepoWifiNetwork() = runBlocking(IMMEDIATE) { val wifiNetwork = WifiNetworkModel.Active( 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 ca30285338c3b..9b6aef76d579d 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 @@ -70,6 +70,7 @@ class WifiViewModelTest : SysuiTestCase() { MockitoAnnotations.initMocks(this) connectivityRepository = FakeConnectivityRepository() wifiRepository = FakeWifiRepository() + wifiRepository.setIsWifiEnabled(true) interactor = WifiInteractor(connectivityRepository, wifiRepository) scope = CoroutineScope(IMMEDIATE) createAndSetViewModel() @@ -85,6 +86,26 @@ class WifiViewModelTest : SysuiTestCase() { // same data for icon, activity, etc. flows. So, most of these tests will test just one of the // instances. There are also some tests that verify all 3 instances received the same data. + @Test + fun wifiIcon_notEnabled_outputsNull() = runBlocking(IMMEDIATE) { + wifiRepository.setIsWifiEnabled(false) + + // Start as non-null so we can verify we got the update + var latest: Icon? = Icon.Resource(0, null) + val job = underTest + .home + .wifiIcon + .onEach { latest = it } + .launchIn(this) + + wifiRepository.setWifiNetwork(WifiNetworkModel.Active(NETWORK_ID, level = 2)) + yield() + + assertThat(latest).isNull() + + job.cancel() + } + @Test fun wifiIcon_forceHidden_outputsNull() = runBlocking(IMMEDIATE) { connectivityRepository.setForceHiddenIcons(setOf(ConnectivitySlot.WIFI)) From 1cb44532d737cf50a092c3e99a26087c7e201f38 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Fri, 23 Sep 2022 14:19:18 +0000 Subject: [PATCH 5/8] [SB Refactor] Add an annotation for the visibility states. Also updates the starting visibility state values to HIDDEN for mobile and wifi. Bug: 238425913 Test: manual: Verified icons still work Test: StatusBarIconViewTest Change-Id: Icd9c2b14f46a148acc2113ac1e20f85924f19e75 --- .../systemui/statusbar/StatusBarIconView.java | 15 ++++++++++++--- .../systemui/statusbar/StatusBarMobileView.java | 6 ++++-- .../systemui/statusbar/StatusBarWifiView.java | 6 ++++-- .../statusbar/StatusIconDisplayable.java | 16 +++++++++++++--- .../wifi/ui/view/ModernStatusBarWifiView.kt | 4 +++- 5 files changed, 36 insertions(+), 11 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarIconView.java b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarIconView.java index 039a3625c70cc..0c3a3c5d03acf 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarIconView.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarIconView.java @@ -22,6 +22,7 @@ import android.animation.Animator; import android.animation.AnimatorListenerAdapter; import android.animation.ObjectAnimator; import android.animation.ValueAnimator; +import android.annotation.IntDef; import android.app.ActivityManager; import android.app.Notification; import android.content.Context; @@ -36,7 +37,6 @@ import android.graphics.Paint; import android.graphics.Rect; import android.graphics.drawable.Drawable; import android.graphics.drawable.Icon; -import android.os.Parcelable; import android.os.Trace; import android.os.UserHandle; import android.service.notification.StatusBarNotification; @@ -60,6 +60,8 @@ import com.android.systemui.statusbar.notification.NotificationIconDozeHelper; import com.android.systemui.statusbar.notification.NotificationUtils; import com.android.systemui.util.drawable.DrawableSize; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; import java.text.NumberFormat; import java.util.ArrayList; import java.util.Arrays; @@ -87,6 +89,10 @@ public class StatusBarIconView extends AnimatedImageView implements StatusIconDi public static final int STATE_DOT = 1; public static final int STATE_HIDDEN = 2; + @Retention(RetentionPolicy.SOURCE) + @IntDef({STATE_ICON, STATE_DOT, STATE_HIDDEN}) + public @interface VisibleState { } + private static final String TAG = "StatusBarIconView"; private static final Property ICON_APPEAR_AMOUNT = new FloatProperty("iconAppearAmount") { @@ -134,6 +140,7 @@ public class StatusBarIconView extends AnimatedImageView implements StatusIconDi private final Paint mDotPaint = new Paint(Paint.ANTI_ALIAS_FLAG); private float mDotRadius; private int mStaticDotRadius; + @StatusBarIconView.VisibleState private int mVisibleState = STATE_ICON; private float mIconAppearAmount = 1.0f; private ObjectAnimator mIconAppearAnimator; @@ -744,11 +751,12 @@ public class StatusBarIconView extends AnimatedImageView implements StatusIconDi } @Override - public void setVisibleState(int state) { + public void setVisibleState(@StatusBarIconView.VisibleState int state) { setVisibleState(state, true /* animate */, null /* endRunnable */); } - public void setVisibleState(int state, boolean animate) { + @Override + public void setVisibleState(@StatusBarIconView.VisibleState int state, boolean animate) { setVisibleState(state, animate, null); } @@ -860,6 +868,7 @@ public class StatusBarIconView extends AnimatedImageView implements StatusIconDi return mIconAppearAmount; } + @StatusBarIconView.VisibleState public int getVisibleState() { return mVisibleState; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarMobileView.java b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarMobileView.java index 25c6dce96b5c8..48c6e273bbb4e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarMobileView.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarMobileView.java @@ -59,7 +59,8 @@ public class StatusBarMobileView extends FrameLayout implements DarkReceiver, private ImageView mOut; private ImageView mMobile, mMobileType, mMobileRoaming; private View mMobileRoamingSpace; - private int mVisibleState = -1; + @StatusBarIconView.VisibleState + private int mVisibleState = STATE_HIDDEN; private DualToneHandler mDualToneHandler; private boolean mForceHidden; @@ -271,7 +272,7 @@ public class StatusBarMobileView extends FrameLayout implements DarkReceiver, } @Override - public void setVisibleState(int state, boolean animate) { + public void setVisibleState(@StatusBarIconView.VisibleState int state, boolean animate) { if (state == mVisibleState) { return; } @@ -312,6 +313,7 @@ public class StatusBarMobileView extends FrameLayout implements DarkReceiver, } @Override + @StatusBarIconView.VisibleState public int getVisibleState() { return mVisibleState; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarWifiView.java b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarWifiView.java index 5aee62e3e89fc..f3e74d92fc8a1 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarWifiView.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarWifiView.java @@ -55,7 +55,8 @@ public class StatusBarWifiView extends BaseStatusBarWifiView implements DarkRece private View mAirplaneSpacer; private WifiIconState mState; private String mSlot; - private int mVisibleState = -1; + @StatusBarIconView.VisibleState + private int mVisibleState = STATE_HIDDEN; public static StatusBarWifiView fromContext(Context context, String slot) { LayoutInflater inflater = LayoutInflater.from(context); @@ -107,7 +108,7 @@ public class StatusBarWifiView extends BaseStatusBarWifiView implements DarkRece } @Override - public void setVisibleState(int state, boolean animate) { + public void setVisibleState(@StatusBarIconView.VisibleState int state, boolean animate) { if (state == mVisibleState) { return; } @@ -131,6 +132,7 @@ public class StatusBarWifiView extends BaseStatusBarWifiView implements DarkRece } @Override + @StatusBarIconView.VisibleState public int getVisibleState() { return mVisibleState; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/StatusIconDisplayable.java b/packages/SystemUI/src/com/android/systemui/statusbar/StatusIconDisplayable.java index d541fae4ed332..cf7283c6442e7 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/StatusIconDisplayable.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/StatusIconDisplayable.java @@ -22,14 +22,24 @@ public interface StatusIconDisplayable extends DarkReceiver { String getSlot(); void setStaticDrawableColor(int color); void setDecorColor(int color); - default void setVisibleState(int state) { + + /** Sets the visible state that this displayable should be. */ + default void setVisibleState(@StatusBarIconView.VisibleState int state) { setVisibleState(state, false); } - void setVisibleState(int state, boolean animate); + + /** + * Sets the visible state that this displayable should be, and whether the change should + * animate. + */ + void setVisibleState(@StatusBarIconView.VisibleState int state, boolean animate); + + /** Returns the current visible state of this displayable. */ + @StatusBarIconView.VisibleState int getVisibleState(); + boolean isIconVisible(); default boolean isIconBlocked() { return false; } } - diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiView.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiView.kt index f225b65179ed1..b874fb10fd1d1 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiView.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiView.kt @@ -22,6 +22,7 @@ import android.util.AttributeSet import android.view.LayoutInflater import com.android.systemui.R import com.android.systemui.statusbar.BaseStatusBarWifiView +import com.android.systemui.statusbar.StatusBarIconView import com.android.systemui.statusbar.StatusBarIconView.STATE_ICON import com.android.systemui.statusbar.phone.StatusBarLocation import com.android.systemui.statusbar.pipeline.wifi.ui.binder.WifiViewBinder @@ -52,10 +53,11 @@ class ModernStatusBarWifiView( // TODO(b/238425913) } - override fun setVisibleState(state: Int, animate: Boolean) { + override fun setVisibleState(@StatusBarIconView.VisibleState state: Int, animate: Boolean) { // TODO(b/238425913) } + @StatusBarIconView.VisibleState override fun getVisibleState(): Int { // TODO(b/238425913) return STATE_ICON From a0fbcf44f17affdce63997860a99d9e6a3a1dd00 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Fri, 23 Sep 2022 14:20:41 +0000 Subject: [PATCH 6/8] [SB Refactor] Connect the old pipeline and new pipeline visibility calculations together for wifi. The _new_ pipeline has the source of truth for whether the wifi icon *should* be visible. But, the _old_ view system has the source of truth for whether the wifi icon *can* be visible (i.e. if there's enough space for it). This CL hooks up those two systems together by having `WifiViewBinder#bind` return a `Binding` interface implementation, similar to `KeyguardButtomAreaViewBinder#bind`. This allows them to communicate with each other and correctly change the view's visibility. Bug: 238425913 Test: manual: Go into airplane mode and verify that the wifi icon disappears completely and there's no empty gap in the status bar. Test: manual: Go into a state where there's not enough room for the wifi icon and verify that just a tinted dot shows up. See demo video in b/238425913#comment30. Test: statusbar.pipeline tests Change-Id: I9d421a4afb503c9327578f777561a4d38bf2fd48 --- packages/SystemUI/res/values/ids.xml | 3 + .../statusbar/StatusIconDisplayable.java | 8 + .../pipeline/wifi/ui/binder/WifiViewBinder.kt | 54 +++++- .../wifi/ui/view/ModernStatusBarWifiView.kt | 55 ++++-- .../wifi/ui/viewmodel/HomeWifiViewModel.kt | 3 +- .../ui/viewmodel/KeyguardWifiViewModel.kt | 3 +- .../viewmodel/LocationBasedWifiViewModel.kt | 3 +- .../wifi/ui/viewmodel/QsWifiViewModel.kt | 3 +- .../wifi/ui/viewmodel/WifiViewModel.kt | 3 +- .../ui/view/ModernStatusBarWifiViewTest.kt | 183 +++++++++++++++++- 10 files changed, 287 insertions(+), 31 deletions(-) diff --git a/packages/SystemUI/res/values/ids.xml b/packages/SystemUI/res/values/ids.xml index 808425435efa7..f22e79722e78b 100644 --- a/packages/SystemUI/res/values/ids.xml +++ b/packages/SystemUI/res/values/ids.xml @@ -131,6 +131,9 @@ + + + diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/StatusIconDisplayable.java b/packages/SystemUI/src/com/android/systemui/statusbar/StatusIconDisplayable.java index cf7283c6442e7..1196211bd6710 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/StatusIconDisplayable.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/StatusIconDisplayable.java @@ -38,7 +38,15 @@ public interface StatusIconDisplayable extends DarkReceiver { @StatusBarIconView.VisibleState int getVisibleState(); + /** + * Returns true if this icon should be visible if there's space, and false otherwise. + * + * Note that this doesn't necessarily mean it *will* be visible. It's possible that there are + * more icons than space, in which case this icon might just show a dot or might be completely + * hidden. {@link #getVisibleState} will return the icon's actual visible status. + */ boolean isIconVisible(); + default boolean isIconBlocked() { return false; } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt index c3a9b90c5a627..273be63eb8a2c 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/binder/WifiViewBinder.kt @@ -26,10 +26,15 @@ import androidx.lifecycle.repeatOnLifecycle import com.android.systemui.R import com.android.systemui.common.ui.binder.IconViewBinder import com.android.systemui.lifecycle.repeatWhenAttached +import com.android.systemui.statusbar.StatusBarIconView +import com.android.systemui.statusbar.StatusBarIconView.STATE_DOT +import com.android.systemui.statusbar.StatusBarIconView.STATE_HIDDEN +import com.android.systemui.statusbar.StatusBarIconView.STATE_ICON import com.android.systemui.statusbar.phone.StatusBarLocation import com.android.systemui.statusbar.pipeline.wifi.ui.viewmodel.LocationBasedWifiViewModel import com.android.systemui.statusbar.pipeline.wifi.ui.viewmodel.WifiViewModel import kotlinx.coroutines.InternalCoroutinesApi +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.collect import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.launch @@ -44,6 +49,19 @@ import kotlinx.coroutines.launch @OptIn(InternalCoroutinesApi::class) object WifiViewBinder { + /** + * Defines interface for an object that acts as the binding between the view and its view-model. + * + * Users of the [WifiViewBinder] class should use this to control the binder after it is bound. + */ + interface Binding { + /** Returns true if the wifi icon should be visible and false otherwise. */ + fun getShouldIconBeVisible(): Boolean + + /** Notifies that the visibility state has changed. */ + fun onVisibilityStateChanged(@StatusBarIconView.VisibleState state: Int) + } + /** * Binds the view to the appropriate view-model based on the given location. The view will * continue to be updated following updates from the view-model. @@ -53,8 +71,8 @@ object WifiViewBinder { view: ViewGroup, wifiViewModel: WifiViewModel, location: StatusBarLocation, - ) { - when (location) { + ): Binding { + return when (location) { StatusBarLocation.HOME -> bind(view, wifiViewModel.home) StatusBarLocation.KEYGUARD -> bind(view, wifiViewModel.keyguard) StatusBarLocation.QS -> bind(view, wifiViewModel.qs) @@ -66,8 +84,10 @@ object WifiViewBinder { private fun bind( view: ViewGroup, viewModel: LocationBasedWifiViewModel, - ) { + ): Binding { + val groupView = view.requireViewById(R.id.wifi_group) val iconView = view.requireViewById(R.id.wifi_signal) + val dotView = view.requireViewById(R.id.status_bar_dot) val activityInView = view.requireViewById(R.id.wifi_in) val activityOutView = view.requireViewById(R.id.wifi_out) val activityContainerView = view.requireViewById(R.id.inout_container) @@ -75,14 +95,21 @@ object WifiViewBinder { view.isVisible = true iconView.isVisible = true + // TODO(b/238425913): We should log this visibility state. + @StatusBarIconView.VisibleState + val visibilityState: MutableStateFlow = MutableStateFlow(STATE_HIDDEN) + view.repeatWhenAttached { repeatOnLifecycle(Lifecycle.State.STARTED) { launch { - viewModel.wifiIcon.distinctUntilChanged().collect { wifiIcon -> - // TODO(b/238425913): Right now, if !isVisible, there's just an empty space - // where the wifi icon would be. We need to pipe isVisible through to - // [ModernStatusBarWifiView.isIconVisible], which is what actually makes - // the view GONE. + visibilityState.collect { visibilityState -> + groupView.isVisible = visibilityState == STATE_ICON + dotView.isVisible = visibilityState == STATE_DOT + } + } + + launch { + viewModel.wifiIcon.collect { wifiIcon -> view.isVisible = wifiIcon != null wifiIcon?.let { IconViewBinder.bind(wifiIcon, iconView) } } @@ -94,6 +121,7 @@ object WifiViewBinder { iconView.imageTintList = tintList activityInView.imageTintList = tintList activityOutView.imageTintList = tintList + dotView.setDecorColor(tint) } } @@ -116,5 +144,15 @@ object WifiViewBinder { } } } + + return object : Binding { + override fun getShouldIconBeVisible(): Boolean { + return viewModel.wifiIcon.value != null + } + + override fun onVisibilityStateChanged(@StatusBarIconView.VisibleState state: Int) { + visibilityState.value = state + } + } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiView.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiView.kt index b874fb10fd1d1..6c616ac7c3b89 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiView.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiView.kt @@ -19,11 +19,13 @@ package com.android.systemui.statusbar.pipeline.wifi.ui.view import android.content.Context import android.graphics.Rect import android.util.AttributeSet +import android.view.Gravity import android.view.LayoutInflater import com.android.systemui.R import com.android.systemui.statusbar.BaseStatusBarWifiView import com.android.systemui.statusbar.StatusBarIconView -import com.android.systemui.statusbar.StatusBarIconView.STATE_ICON +import com.android.systemui.statusbar.StatusBarIconView.STATE_DOT +import com.android.systemui.statusbar.StatusBarIconView.STATE_HIDDEN import com.android.systemui.statusbar.phone.StatusBarLocation import com.android.systemui.statusbar.pipeline.wifi.ui.binder.WifiViewBinder import com.android.systemui.statusbar.pipeline.wifi.ui.viewmodel.WifiViewModel @@ -38,6 +40,17 @@ class ModernStatusBarWifiView( ) : BaseStatusBarWifiView(context, attrs) { private lateinit var slot: String + private lateinit var binding: WifiViewBinder.Binding + + @StatusBarIconView.VisibleState + private var iconVisibleState: Int = STATE_HIDDEN + set(value) { + if (field == value) { + return + } + field = value + binding.onVisibilityStateChanged(value) + } override fun onDarkChanged(areas: ArrayList?, darkIntensity: Float, tint: Int) { // TODO(b/238425913) @@ -54,23 +67,44 @@ class ModernStatusBarWifiView( } override fun setVisibleState(@StatusBarIconView.VisibleState state: Int, animate: Boolean) { - // TODO(b/238425913) + iconVisibleState = state } @StatusBarIconView.VisibleState override fun getVisibleState(): Int { - // TODO(b/238425913) - return STATE_ICON + return iconVisibleState } override fun isIconVisible(): Boolean { - // TODO(b/238425913) - return true + return binding.getShouldIconBeVisible() } - /** Set the slot name for this view. */ - private fun setSlot(slotName: String) { - this.slot = slotName + private fun initView( + slotName: String, + wifiViewModel: WifiViewModel, + location: StatusBarLocation, + ) { + slot = slotName + initDotView() + binding = WifiViewBinder.bind(this, wifiViewModel, location) + } + + // Mostly duplicated from [com.android.systemui.statusbar.StatusBarWifiView]. + private fun initDotView() { + // TODO(b/238425913): Could we just have this dot view be part of + // R.layout.new_status_bar_wifi_group with a dot drawable so we don't need to inflate it + // manually? Would that not work with animations? + val dotView = StatusBarIconView(mContext, slot, null).also { + it.id = R.id.status_bar_dot + // Hard-code this view to always be in the DOT state so that whenever it's visible it + // will show a dot + it.visibleState = STATE_DOT + } + + val width = mContext.resources.getDimensionPixelSize(R.dimen.status_bar_icon_size) + val lp = LayoutParams(width, width) + lp.gravity = Gravity.CENTER_VERTICAL or Gravity.START + addView(dotView, lp) } companion object { @@ -89,8 +123,7 @@ class ModernStatusBarWifiView( LayoutInflater.from(context).inflate(R.layout.new_status_bar_wifi_group, null) as ModernStatusBarWifiView ).also { - it.setSlot(slot) - WifiViewBinder.bind(it, wifiViewModel, location) + it.initView(slot, wifiViewModel, location) } } } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/HomeWifiViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/HomeWifiViewModel.kt index 0847e6214337f..871b395d09964 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/HomeWifiViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/HomeWifiViewModel.kt @@ -20,6 +20,7 @@ import android.graphics.Color import com.android.systemui.common.shared.model.Icon import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.StateFlow /** * A view model for the wifi icon shown on the "home" page (aka, when the device is unlocked and not @@ -27,7 +28,7 @@ import kotlinx.coroutines.flow.Flow */ class HomeWifiViewModel( statusBarPipelineFlags: StatusBarPipelineFlags, - wifiIcon: Flow, + wifiIcon: StateFlow, isActivityInViewVisible: Flow, isActivityOutViewVisible: Flow, isActivityContainerVisible: Flow, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/KeyguardWifiViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/KeyguardWifiViewModel.kt index 3f7c8e13d7f72..be1f3f2194bc5 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/KeyguardWifiViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/KeyguardWifiViewModel.kt @@ -20,11 +20,12 @@ import android.graphics.Color import com.android.systemui.common.shared.model.Icon import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.StateFlow /** A view model for the wifi icon shown on keyguard (lockscreen). */ class KeyguardWifiViewModel( statusBarPipelineFlags: StatusBarPipelineFlags, - wifiIcon: Flow, + wifiIcon: StateFlow, isActivityInViewVisible: Flow, isActivityOutViewVisible: Flow, isActivityContainerVisible: Flow, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/LocationBasedWifiViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/LocationBasedWifiViewModel.kt index d34ba88cde1a8..7243acfbd56d9 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/LocationBasedWifiViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/LocationBasedWifiViewModel.kt @@ -20,6 +20,7 @@ import android.graphics.Color import com.android.systemui.common.shared.model.Icon import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.flowOf /** @@ -33,7 +34,7 @@ abstract class LocationBasedWifiViewModel( debugTint: Int, /** The wifi icon that should be displayed. Null if we shouldn't display any icon. */ - val wifiIcon: Flow, + val wifiIcon: StateFlow, /** True if the activity in view should be visible. */ val isActivityInViewVisible: Flow, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/QsWifiViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/QsWifiViewModel.kt index 8f6c26a6620fa..d640d33eb316e 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/QsWifiViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/QsWifiViewModel.kt @@ -20,11 +20,12 @@ import android.graphics.Color import com.android.systemui.common.shared.model.Icon import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.StateFlow /** A view model for the wifi icon shown in quick settings (when the shade is pulled down). */ class QsWifiViewModel( statusBarPipelineFlags: StatusBarPipelineFlags, - wifiIcon: Flow, + wifiIcon: StateFlow, isActivityInViewVisible: Flow, isActivityOutViewVisible: Flow, isActivityContainerVisible: Flow, 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 13fd922720f66..465f5097235e4 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 @@ -41,6 +41,7 @@ import javax.inject.Inject import kotlinx.coroutines.CoroutineScope 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 @@ -120,7 +121,7 @@ constructor( .stateIn(scope, started = SharingStarted.WhileSubscribed(), initialValue = null) /** The wifi icon that should be displayed. Null if we shouldn't display any icon. */ - private val wifiIcon: Flow = + private val wifiIcon: StateFlow = combine( interactor.isEnabled, interactor.isForceHidden, diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt index cbb4b7e051761..a93b6b28e3b3c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt @@ -16,39 +16,208 @@ package com.android.systemui.statusbar.pipeline.wifi.ui.view +import android.testing.AndroidTestingRunner +import android.testing.TestableLooper import android.testing.TestableLooper.RunWithLooper +import android.testing.ViewUtils +import android.view.View import androidx.test.filters.SmallTest +import com.android.systemui.R import com.android.systemui.SysuiTestCase import com.android.systemui.lifecycle.InstantTaskExecutorRule +import com.android.systemui.statusbar.StatusBarIconView.STATE_DOT +import com.android.systemui.statusbar.StatusBarIconView.STATE_HIDDEN +import com.android.systemui.statusbar.StatusBarIconView.STATE_ICON import com.android.systemui.statusbar.phone.StatusBarLocation -import com.android.systemui.util.Assert -import com.android.systemui.util.mockito.mock +import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags +import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger +import com.android.systemui.statusbar.pipeline.shared.data.repository.FakeConnectivityRepository +import com.android.systemui.statusbar.pipeline.wifi.data.repository.FakeWifiRepository +import com.android.systemui.statusbar.pipeline.wifi.domain.interactor.WifiInteractor +import com.android.systemui.statusbar.pipeline.wifi.shared.WifiConstants +import com.android.systemui.statusbar.pipeline.wifi.ui.viewmodel.WifiViewModel import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import org.junit.Before import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith -import org.junit.runners.JUnit4 +import org.mockito.Mock +import org.mockito.MockitoAnnotations @SmallTest -@RunWith(JUnit4::class) -@RunWithLooper +@RunWith(AndroidTestingRunner::class) +@RunWithLooper(setAsMainLooper = true) class ModernStatusBarWifiViewTest : SysuiTestCase() { + private lateinit var testableLooper: TestableLooper + + @Mock + private lateinit var statusBarPipelineFlags: StatusBarPipelineFlags + @Mock + private lateinit var logger: ConnectivityPipelineLogger + @Mock + private lateinit var constants: WifiConstants + private lateinit var connectivityRepository: FakeConnectivityRepository + private lateinit var wifiRepository: FakeWifiRepository + private lateinit var interactor: WifiInteractor + private lateinit var viewModel: WifiViewModel + private lateinit var scope: CoroutineScope + @JvmField @Rule val instantTaskExecutor = InstantTaskExecutorRule() @Before fun setUp() { - Assert.setTestThread(Thread.currentThread()) + MockitoAnnotations.initMocks(this) + testableLooper = TestableLooper.get(this) + + connectivityRepository = FakeConnectivityRepository() + wifiRepository = FakeWifiRepository() + wifiRepository.setIsWifiEnabled(true) + interactor = WifiInteractor(connectivityRepository, wifiRepository) + scope = CoroutineScope(Dispatchers.Unconfined) + viewModel = WifiViewModel( + constants, context, logger, interactor, scope, statusBarPipelineFlags + ) } @Test fun constructAndBind_hasCorrectSlot() { val view = ModernStatusBarWifiView.constructAndBind( - context, "slotName", mock(), StatusBarLocation.HOME + context, "slotName", viewModel, StatusBarLocation.HOME ) assertThat(view.slot).isEqualTo("slotName") } + + @Test + fun getVisibleState_icon_returnsIcon() { + val view = ModernStatusBarWifiView.constructAndBind( + context, SLOT_NAME, viewModel, StatusBarLocation.HOME + ) + + view.setVisibleState(STATE_ICON, /* animate= */ false) + + assertThat(view.visibleState).isEqualTo(STATE_ICON) + } + + @Test + fun getVisibleState_dot_returnsDot() { + val view = ModernStatusBarWifiView.constructAndBind( + context, SLOT_NAME, viewModel, StatusBarLocation.HOME + ) + + view.setVisibleState(STATE_DOT, /* animate= */ false) + + assertThat(view.visibleState).isEqualTo(STATE_DOT) + } + + @Test + fun getVisibleState_hidden_returnsHidden() { + val view = ModernStatusBarWifiView.constructAndBind( + context, SLOT_NAME, viewModel, StatusBarLocation.HOME + ) + + view.setVisibleState(STATE_HIDDEN, /* animate= */ false) + + assertThat(view.visibleState).isEqualTo(STATE_HIDDEN) + } + + // Note: The following tests are more like integration tests, since they stand up a full + // [WifiViewModel] and test the interactions between the view, view-binder, and view-model. + + @Test + fun setVisibleState_icon_iconShownDotHidden() { + val view = ModernStatusBarWifiView.constructAndBind( + context, SLOT_NAME, viewModel, StatusBarLocation.HOME + ) + + view.setVisibleState(STATE_ICON, /* animate= */ false) + + ViewUtils.attachView(view) + testableLooper.processAllMessages() + + assertThat(view.getIconGroupView().visibility).isEqualTo(View.VISIBLE) + assertThat(view.getDotView().visibility).isEqualTo(View.GONE) + + ViewUtils.detachView(view) + } + + @Test + fun setVisibleState_dot_iconHiddenDotShown() { + val view = ModernStatusBarWifiView.constructAndBind( + context, SLOT_NAME, viewModel, StatusBarLocation.HOME + ) + + view.setVisibleState(STATE_DOT, /* animate= */ false) + + ViewUtils.attachView(view) + testableLooper.processAllMessages() + + assertThat(view.getIconGroupView().visibility).isEqualTo(View.GONE) + assertThat(view.getDotView().visibility).isEqualTo(View.VISIBLE) + + ViewUtils.detachView(view) + } + + @Test + fun setVisibleState_hidden_iconAndDotHidden() { + val view = ModernStatusBarWifiView.constructAndBind( + context, SLOT_NAME, viewModel, StatusBarLocation.HOME + ) + + view.setVisibleState(STATE_HIDDEN, /* animate= */ false) + + ViewUtils.attachView(view) + testableLooper.processAllMessages() + + assertThat(view.getIconGroupView().visibility).isEqualTo(View.GONE) + assertThat(view.getDotView().visibility).isEqualTo(View.GONE) + + ViewUtils.detachView(view) + } + + @Test + fun isIconVisible_notEnabled_outputsFalse() { + wifiRepository.setIsWifiEnabled(false) + + val view = ModernStatusBarWifiView.constructAndBind( + context, SLOT_NAME, viewModel, StatusBarLocation.HOME + ) + + ViewUtils.attachView(view) + testableLooper.processAllMessages() + + assertThat(view.isIconVisible).isFalse() + + ViewUtils.detachView(view) + } + + @Test + fun isIconVisible_enabled_outputsTrue() { + wifiRepository.setIsWifiEnabled(true) + + val view = ModernStatusBarWifiView.constructAndBind( + context, SLOT_NAME, viewModel, StatusBarLocation.HOME + ) + + ViewUtils.attachView(view) + testableLooper.processAllMessages() + + assertThat(view.isIconVisible).isTrue() + + ViewUtils.detachView(view) + } + + private fun View.getIconGroupView(): View { + return this.requireViewById(R.id.wifi_group) + } + + private fun View.getDotView(): View { + return this.requireViewById(R.id.status_bar_dot) + } } + +private const val SLOT_NAME = "TestSlotName" From 022a29820b377a78e02d10e441f3d11a92e507f2 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Mon, 26 Sep 2022 18:03:44 +0000 Subject: [PATCH 7/8] [SB Refactor] Listen to the WIFI_STATE_CHANGED_ACTION broadcasts and re-fetch `isWifiEnabled` whenever it happens. Bug: 238425913 Test: manual: Go into airplane mode and verify that we get logs for the WIFI_STATE_CHANGED_ACTION intent and for `enabled` being updated; leave airplane mode and verify the same Test: statusbar.pipeline tests Change-Id: I1eba794e98a288683bc4cfe0405a98c48a2b2c03 --- .../shared/ConnectivityPipelineLogger.kt | 38 ++++++ .../wifi/data/repository/WifiRepository.kt | 32 +++-- .../shared/ConnectivityPipelineLoggerTest.kt | 37 ++++++ .../data/repository/WifiRepositoryImplTest.kt | 125 ++++++++++++++---- 4 files changed, 190 insertions(+), 42 deletions(-) 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 3a7ff167af18d..dbb1aa54d8ee6 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 @@ -31,6 +31,20 @@ import kotlinx.coroutines.flow.onEach class ConnectivityPipelineLogger @Inject constructor( @StatusBarConnectivityLog private val buffer: LogBuffer, ) { + /** + * Logs a change in one of the **raw inputs** to the connectivity pipeline. + * + * 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" } + ) + } + /** * Logs a change in one of the **raw inputs** to the connectivity pipeline. */ @@ -127,6 +141,30 @@ class ConnectivityPipelineLogger @Inject constructor( companion object { const val SB_LOGGING_TAG = "SbConnectivity" + /** + * Log a change in one of the **inputs** to the connectivity pipeline. + */ + fun Flow.logInputChange( + logger: ConnectivityPipelineLogger, + inputParamName: String, + ): Flow { + return this.onEach { logger.logInputChange(inputParamName) } + } + + /** + * 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. + */ + fun Flow.logInputChange( + logger: ConnectivityPipelineLogger, + inputParamName: String, + prettyPrint: (T) -> String = { it.toString() } + ): Flow { + return this.onEach {logger.logInputChange(inputParamName, prettyPrint(it)) } + } + /** * Log a change in one of the **outputs** to the connectivity pipeline. * diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt index 1400032ea0d31..681cf7254ae75 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepository.kt @@ -17,6 +17,7 @@ package com.android.systemui.statusbar.pipeline.wifi.data.repository import android.annotation.SuppressLint +import android.content.IntentFilter import android.net.ConnectivityManager import android.net.Network import android.net.NetworkCapabilities @@ -30,12 +31,14 @@ import android.net.wifi.WifiManager import android.net.wifi.WifiManager.TrafficStateCallback import android.util.Log import com.android.settingslib.Utils +import com.android.systemui.broadcast.BroadcastDispatcher import com.android.systemui.common.coroutine.ConflatedCallbackFlow.conflatedCallbackFlow import com.android.systemui.dagger.SysUISingleton import com.android.systemui.dagger.qualifiers.Application import com.android.systemui.dagger.qualifiers.Main import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.SB_LOGGING_TAG +import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logInputChange import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logOutputChange import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.shared.model.WifiActivityModel @@ -44,6 +47,7 @@ import javax.inject.Inject import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.channels.awaitClose +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted @@ -52,6 +56,7 @@ import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.mapLatest +import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.stateIn /** Provides data related to the wifi state. */ @@ -67,10 +72,12 @@ interface WifiRepository { } /** Real implementation of [WifiRepository]. */ +@Suppress("EXPERIMENTAL_IS_NOT_ENABLED") @OptIn(ExperimentalCoroutinesApi::class) @SysUISingleton @SuppressLint("MissingPermission") class WifiRepositoryImpl @Inject constructor( + broadcastDispatcher: BroadcastDispatcher, connectivityManager: ConnectivityManager, logger: ConnectivityPipelineLogger, @Main mainExecutor: Executor, @@ -78,23 +85,22 @@ class WifiRepositoryImpl @Inject constructor( wifiManager: WifiManager?, ) : WifiRepository { - /** - * A flow that emits [Unit] whenever the wifi state may have changed. - * - * Because [WifiManager] doesn't expose a wifi state change listener, we do it internally by - * emitting to this flow whenever we think the state may have changed. - * - * TODO(b/238425913): We also need to emit to this flow whenever the WIFI_STATE_CHANGED_ACTION - * intent is triggered. - */ - private val _wifiStateChangeEvents: MutableSharedFlow = + private val wifiStateChangeEvents: Flow = broadcastDispatcher.broadcastFlow( + IntentFilter(WifiManager.WIFI_STATE_CHANGED_ACTION) + ) + .logInputChange(logger, "WIFI_STATE_CHANGED_ACTION intent") + + private val wifiNetworkChangeEvents: MutableSharedFlow = MutableSharedFlow(extraBufferCapacity = 1) override val isWifiEnabled: StateFlow = if (wifiManager == null) { MutableStateFlow(false).asStateFlow() } else { - _wifiStateChangeEvents + // Because [WifiManager] doesn't expose a wifi enabled change listener, we do it + // internally by fetching [WifiManager.isWifiEnabled] whenever we think the state may + // have changed. + merge(wifiNetworkChangeEvents, wifiStateChangeEvents) .mapLatest { wifiManager.isWifiEnabled } .distinctUntilChanged() .logOutputChange(logger, "enabled") @@ -115,7 +121,7 @@ class WifiRepositoryImpl @Inject constructor( ) { logger.logOnCapabilitiesChanged(network, networkCapabilities) - _wifiStateChangeEvents.tryEmit(Unit) + wifiNetworkChangeEvents.tryEmit(Unit) val wifiInfo = networkCapabilitiesToWifiInfo(networkCapabilities) if (wifiInfo?.isPrimary == true) { @@ -138,7 +144,7 @@ class WifiRepositoryImpl @Inject constructor( override fun onLost(network: Network) { logger.logOnLost(network) - _wifiStateChangeEvents.tryEmit(Unit) + wifiNetworkChangeEvents.tryEmit(Unit) val wifi = currentWifi if (wifi is WifiNetworkModel.Active && wifi.networkId == network.getNetId()) { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLoggerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLoggerTest.kt index d3d8d542e0785..0e75c74ef6f5b 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLoggerTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityPipelineLoggerTest.kt @@ -23,6 +23,7 @@ import com.android.systemui.SysuiTestCase import com.android.systemui.dump.DumpManager import com.android.systemui.log.LogBufferFactory import com.android.systemui.log.LogcatEchoTracker +import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logInputChange import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logOutputChange import com.google.common.truth.Truth.assertThat import java.io.PrintWriter @@ -89,6 +90,42 @@ class ConnectivityPipelineLoggerTest : SysuiTestCase() { job.cancel() } + @Test + fun logInputChange_unit_printsInputName() = runBlocking(IMMEDIATE) { + val flow: Flow = flowOf(Unit, Unit) + + val job = flow + .logInputChange(logger, "testInputs") + .launchIn(this) + + val stringWriter = StringWriter() + buffer.dump(PrintWriter(stringWriter), tailLength = 0) + val actualString = stringWriter.toString() + + assertThat(actualString).contains("testInputs") + + job.cancel() + } + + @Test + fun logInputChange_any_printsValuesAndNulls() = runBlocking(IMMEDIATE) { + val flow: Flow = flowOf(null, 2, "threeString") + + val job = flow + .logInputChange(logger, "testInputs") + .launchIn(this) + + val stringWriter = StringWriter() + buffer.dump(PrintWriter(stringWriter), tailLength = 0) + val actualString = stringWriter.toString() + + assertThat(actualString).contains("null") + assertThat(actualString).contains("2") + assertThat(actualString).contains("threeString") + + job.cancel() + } + companion object { private const val NET_1_ID = 100 private val NET_1 = com.android.systemui.util.mockito.mock().also { diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepositoryImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepositoryImplTest.kt index c962315adb51b..0ba0bd623c391 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepositoryImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/data/repository/WifiRepositoryImplTest.kt @@ -28,6 +28,7 @@ import android.net.wifi.WifiManager import android.net.wifi.WifiManager.TrafficStateCallback import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase +import com.android.systemui.broadcast.BroadcastDispatcher import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.data.repository.WifiRepositoryImpl.Companion.ACTIVITY_DEFAULT @@ -37,6 +38,7 @@ import com.android.systemui.util.concurrency.FakeExecutor import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.argumentCaptor import com.android.systemui.util.mockito.mock +import com.android.systemui.util.mockito.nullable import com.android.systemui.util.time.FakeSystemClock import com.google.common.truth.Truth.assertThat import java.util.concurrent.Executor @@ -44,23 +46,28 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.cancel +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.runBlocking import org.junit.After import org.junit.Before import org.junit.Test +import org.mockito.ArgumentMatchers.anyInt import org.mockito.Mock import org.mockito.Mockito.verify import org.mockito.Mockito.`when` as whenever import org.mockito.MockitoAnnotations +@Suppress("EXPERIMENTAL_IS_NOT_ENABLED") @OptIn(ExperimentalCoroutinesApi::class) @SmallTest class WifiRepositoryImplTest : SysuiTestCase() { private lateinit var underTest: WifiRepositoryImpl + @Mock private lateinit var broadcastDispatcher: BroadcastDispatcher @Mock private lateinit var logger: ConnectivityPipelineLogger @Mock private lateinit var connectivityManager: ConnectivityManager @Mock private lateinit var wifiManager: WifiManager @@ -70,16 +77,17 @@ class WifiRepositoryImplTest : SysuiTestCase() { @Before fun setUp() { MockitoAnnotations.initMocks(this) + whenever( + broadcastDispatcher.broadcastFlow( + any(), + nullable(), + anyInt(), + nullable(), + ) + ).thenReturn(flowOf(Unit)) executor = FakeExecutor(FakeSystemClock()) scope = CoroutineScope(IMMEDIATE) - - underTest = WifiRepositoryImpl( - connectivityManager, - logger, - executor, - scope, - wifiManager, - ) + underTest = createRepo() } @After @@ -89,13 +97,7 @@ class WifiRepositoryImplTest : SysuiTestCase() { @Test fun isWifiEnabled_nullWifiManager_getsFalse() = runBlocking(IMMEDIATE) { - underTest = WifiRepositoryImpl( - connectivityManager, - logger, - executor, - scope, - wifiManager = null, - ) + underTest = createRepo(wifiManagerToUse = null) assertThat(underTest.isWifiEnabled.value).isFalse() } @@ -104,13 +106,7 @@ class WifiRepositoryImplTest : SysuiTestCase() { fun isWifiEnabled_initiallyGetsWifiManagerValue() = runBlocking(IMMEDIATE) { whenever(wifiManager.isWifiEnabled).thenReturn(true) - underTest = WifiRepositoryImpl( - connectivityManager, - logger, - executor, - scope, - wifiManager - ) + underTest = createRepo() assertThat(underTest.isWifiEnabled.value).isTrue() } @@ -159,6 +155,72 @@ class WifiRepositoryImplTest : SysuiTestCase() { enabledJob.cancel() } + @Test + fun isWifiEnabled_intentsReceived_valueUpdated() = runBlocking(IMMEDIATE) { + val intentFlow = MutableSharedFlow() + whenever( + broadcastDispatcher.broadcastFlow( + any(), + nullable(), + anyInt(), + nullable(), + ) + ).thenReturn(intentFlow) + underTest = createRepo() + + val job = underTest.isWifiEnabled.launchIn(this) + + whenever(wifiManager.isWifiEnabled).thenReturn(true) + intentFlow.emit(Unit) + + assertThat(underTest.isWifiEnabled.value).isTrue() + + whenever(wifiManager.isWifiEnabled).thenReturn(false) + intentFlow.emit(Unit) + + assertThat(underTest.isWifiEnabled.value).isFalse() + + job.cancel() + } + + @Test + fun isWifiEnabled_bothIntentAndNetworkUpdates_valueAlwaysUpdated() = runBlocking(IMMEDIATE) { + val intentFlow = MutableSharedFlow() + whenever( + broadcastDispatcher.broadcastFlow( + any(), + nullable(), + anyInt(), + nullable(), + ) + ).thenReturn(intentFlow) + underTest = createRepo() + + val networkJob = underTest.wifiNetwork.launchIn(this) + val enabledJob = underTest.isWifiEnabled.launchIn(this) + + whenever(wifiManager.isWifiEnabled).thenReturn(false) + intentFlow.emit(Unit) + assertThat(underTest.isWifiEnabled.value).isFalse() + + whenever(wifiManager.isWifiEnabled).thenReturn(true) + getNetworkCallback().onLost(NETWORK) + assertThat(underTest.isWifiEnabled.value).isTrue() + + whenever(wifiManager.isWifiEnabled).thenReturn(false) + getNetworkCallback().onCapabilitiesChanged( + NETWORK, createWifiNetworkCapabilities(PRIMARY_WIFI_INFO) + ) + assertThat(underTest.isWifiEnabled.value).isFalse() + + whenever(wifiManager.isWifiEnabled).thenReturn(true) + intentFlow.emit(Unit) + assertThat(underTest.isWifiEnabled.value).isTrue() + + networkJob.cancel() + enabledJob.cancel() + } + @Test fun wifiNetwork_initiallyGetsDefault() = runBlocking(IMMEDIATE) { var latest: WifiNetworkModel? = null @@ -581,13 +643,7 @@ class WifiRepositoryImplTest : SysuiTestCase() { @Test fun wifiActivity_nullWifiManager_receivesDefault() = runBlocking(IMMEDIATE) { - underTest = WifiRepositoryImpl( - connectivityManager, - logger, - executor, - scope, - wifiManager = null, - ) + underTest = createRepo(wifiManagerToUse = null) var latest: WifiActivityModel? = null val job = underTest @@ -666,6 +722,17 @@ class WifiRepositoryImplTest : SysuiTestCase() { job.cancel() } + private fun createRepo(wifiManagerToUse: WifiManager? = wifiManager): WifiRepositoryImpl { + return WifiRepositoryImpl( + broadcastDispatcher, + connectivityManager, + logger, + executor, + scope, + wifiManagerToUse, + ) + } + private fun getTrafficStateCallback(): TrafficStateCallback { val callbackCaptor = argumentCaptor() verify(wifiManager).registerTrafficStateCallback(any(), callbackCaptor.capture()) From 2dc080cbf768fe98b5b6579dfc8909b3da7029ce Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Tue, 27 Sep 2022 17:28:50 +0000 Subject: [PATCH 8/8] [SB Refactor] Only show wifi icon if (1) the network is active and validated; or (2) we're configured to always show it when enabled. This matches part of the logic in WifiSignalController#notifyListenersForNonCarrierWifi.wifiVisible. Test: manual: With alwaysShow config off, verify that the wifi icon only appears when we're fully connected to a network Test: manual: With alwaysShow config on, verify that the wifi icon always appears (unless wifi is disabled) Test: statusbar.pipeline tests Bug: 238425913 Change-Id: I723cfc0435265699f98b5b1adeb4cb9bbfaf2929 --- .../pipeline/wifi/shared/WifiConstants.kt | 5 + .../wifi/ui/viewmodel/WifiViewModel.kt | 104 +++++++++--------- .../ui/view/ModernStatusBarWifiViewTest.kt | 8 ++ .../wifi/ui/viewmodel/WifiViewModelTest.kt | 70 +++++++++++- 4 files changed, 133 insertions(+), 54 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 a19d1bdd8e628..0eb4b0de9f6b6 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 @@ -41,9 +41,14 @@ class WifiConstants @Inject constructor( /** 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 465f5097235e4..47347a2666f93 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 @@ -72,68 +72,70 @@ constructor( @Application private val scope: CoroutineScope, statusBarPipelineFlags: StatusBarPipelineFlags, ) { - /** The drawable resource ID to use for the wifi icon. Null if we shouldn't display any icon. */ + /** + * Returns the drawable resource ID to use for the wifi icon based on the given network. + * Null if we can't compute the icon. + */ @DrawableRes - private val iconResId: Flow = - interactor.wifiNetwork - .map { - when (it) { - is WifiNetworkModel.CarrierMerged -> null - is WifiNetworkModel.Inactive -> WIFI_NO_NETWORK - is WifiNetworkModel.Active -> - when { - it.level == null -> null - it.isValidated -> WIFI_FULL_ICONS[it.level] - else -> WIFI_NO_INTERNET_ICONS[it.level] - } + private fun WifiNetworkModel.iconResId(): Int? { + return when (this) { + is WifiNetworkModel.CarrierMerged -> null + is WifiNetworkModel.Inactive -> WIFI_NO_NETWORK + is WifiNetworkModel.Active -> + when { + this.level == null -> null + this.isValidated -> WIFI_FULL_ICONS[this.level] + else -> WIFI_NO_INTERNET_ICONS[this.level] } - } - .stateIn(scope, started = SharingStarted.WhileSubscribed(), initialValue = null) + } + } - /** The content description for the wifi icon. */ - private val contentDescription: Flow = - interactor.wifiNetwork - .map { - when (it) { - is WifiNetworkModel.CarrierMerged -> null - is WifiNetworkModel.Inactive -> - ContentDescription.Loaded( - "${context.getString(WIFI_NO_CONNECTION)}," + - context.getString(NO_INTERNET) - ) - is WifiNetworkModel.Active -> - when (it.level) { - null -> null - else -> { - val levelDesc = - context.getString(WIFI_CONNECTION_STRENGTH[it.level]) - when { - it.isValidated -> ContentDescription.Loaded(levelDesc) - else -> - ContentDescription.Loaded( - "$levelDesc,${context.getString(NO_INTERNET)}" - ) - } - } + /** + * Returns the content description for the wifi icon based on the given network. + * Null if we can't compute the content description. + */ + private fun WifiNetworkModel.contentDescription(): ContentDescription? { + return when (this) { + is WifiNetworkModel.CarrierMerged -> null + is WifiNetworkModel.Inactive -> + ContentDescription.Loaded( + "${context.getString(WIFI_NO_CONNECTION)},${context.getString(NO_INTERNET)}" + ) + is WifiNetworkModel.Active -> + when (this.level) { + null -> null + else -> { + val levelDesc = context.getString(WIFI_CONNECTION_STRENGTH[this.level]) + when { + this.isValidated -> ContentDescription.Loaded(levelDesc) + else -> + ContentDescription.Loaded( + "$levelDesc,${context.getString(NO_INTERNET)}" + ) } + } } - } - .stateIn(scope, started = SharingStarted.WhileSubscribed(), initialValue = null) + } + } /** The wifi icon that should be displayed. Null if we shouldn't display any icon. */ private val wifiIcon: StateFlow = combine( interactor.isEnabled, interactor.isForceHidden, - iconResId, - contentDescription, - ) { isEnabled, isForceHidden, iconResId, contentDescription -> - when { - !isEnabled || - isForceHidden || - iconResId == null || - iconResId <= 0 -> null - else -> Icon.Resource(iconResId, contentDescription) + interactor.wifiNetwork, + ) { isEnabled, isForceHidden, wifiNetwork -> + if (!isEnabled || isForceHidden || wifiNetwork is WifiNetworkModel.CarrierMerged) { + return@combine null + } + + val iconResId = wifiNetwork.iconResId() ?: return@combine null + val icon = Icon.Resource(iconResId, wifiNetwork.contentDescription()) + + return@combine when { + constants.alwaysShowIconIfEnabled -> icon + wifiNetwork is WifiNetworkModel.Active && wifiNetwork.isValidated -> icon + else -> null } } .stateIn(scope, started = SharingStarted.WhileSubscribed(), initialValue = null) diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt index a93b6b28e3b3c..c577db8c44607 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt @@ -32,6 +32,7 @@ import com.android.systemui.statusbar.phone.StatusBarLocation import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger import com.android.systemui.statusbar.pipeline.shared.data.repository.FakeConnectivityRepository +import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel import com.android.systemui.statusbar.pipeline.wifi.data.repository.FakeWifiRepository import com.android.systemui.statusbar.pipeline.wifi.domain.interactor.WifiInteractor import com.android.systemui.statusbar.pipeline.wifi.shared.WifiConstants @@ -182,6 +183,9 @@ class ModernStatusBarWifiViewTest : SysuiTestCase() { @Test fun isIconVisible_notEnabled_outputsFalse() { wifiRepository.setIsWifiEnabled(false) + wifiRepository.setWifiNetwork( + WifiNetworkModel.Active(NETWORK_ID, isValidated = true, level = 2) + ) val view = ModernStatusBarWifiView.constructAndBind( context, SLOT_NAME, viewModel, StatusBarLocation.HOME @@ -198,6 +202,9 @@ class ModernStatusBarWifiViewTest : SysuiTestCase() { @Test fun isIconVisible_enabled_outputsTrue() { wifiRepository.setIsWifiEnabled(true) + wifiRepository.setWifiNetwork( + WifiNetworkModel.Active(NETWORK_ID, isValidated = true, level = 2) + ) val view = ModernStatusBarWifiView.constructAndBind( context, SLOT_NAME, viewModel, StatusBarLocation.HOME @@ -221,3 +228,4 @@ class ModernStatusBarWifiViewTest : SysuiTestCase() { } private const val SLOT_NAME = "TestSlotName" +private const val NETWORK_ID = 200 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 9b6aef76d579d..063072f7d7634 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 @@ -51,6 +51,7 @@ import org.mockito.Mock import org.mockito.Mockito.`when` as whenever import org.mockito.MockitoAnnotations +@Suppress("EXPERIMENTAL_IS_NOT_ENABLED") @OptIn(ExperimentalCoroutinesApi::class) @SmallTest class WifiViewModelTest : SysuiTestCase() { @@ -137,7 +138,9 @@ class WifiViewModelTest : SysuiTestCase() { .onEach { latest = it } .launchIn(this) - wifiRepository.setWifiNetwork(WifiNetworkModel.Active(NETWORK_ID, level = 2)) + wifiRepository.setWifiNetwork( + WifiNetworkModel.Active(NETWORK_ID, isValidated = true, level = 2) + ) yield() assertThat(latest).isInstanceOf(Icon.Resource::class.java) @@ -146,7 +149,31 @@ class WifiViewModelTest : SysuiTestCase() { } @Test - fun wifiIcon_inactiveNetwork_outputsNoNetworkIcon() = runBlocking(IMMEDIATE) { + fun wifiIcon_inactiveNetwork_alwaysShowFalse_outputsNull() = runBlocking(IMMEDIATE) { + whenever(constants.alwaysShowIconIfEnabled).thenReturn(false) + createAndSetViewModel() + + // Start as non-null so we can verify we got the update + var latest: Icon? = Icon.Resource(0, null) + val job = underTest + .home + .wifiIcon + .onEach { latest = it } + .launchIn(this) + + wifiRepository.setWifiNetwork(WifiNetworkModel.Inactive) + yield() + + assertThat(latest).isNull() + + job.cancel() + } + + @Test + fun wifiIcon_inactiveNetwork_alwaysShowTrue_outputsNoNetworkIcon() = runBlocking(IMMEDIATE) { + whenever(constants.alwaysShowIconIfEnabled).thenReturn(true) + createAndSetViewModel() + var latest: Icon? = null val job = underTest .home @@ -170,6 +197,10 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun wifiIcon_carrierMergedNetwork_outputsNull() = runBlocking(IMMEDIATE) { + // Even when we should always show the icon + whenever(constants.alwaysShowIconIfEnabled).thenReturn(true) + createAndSetViewModel() + var latest: Icon? = Icon.Resource(0, null) val job = underTest .home @@ -177,9 +208,11 @@ class WifiViewModelTest : SysuiTestCase() { .onEach { latest = it } .launchIn(this) + // WHEN we have a carrier merged network wifiRepository.setWifiNetwork(WifiNetworkModel.CarrierMerged) yield() + // THEN we override the alwaysShow boolean and still don't show the icon assertThat(latest).isNull() job.cancel() @@ -187,6 +220,10 @@ class WifiViewModelTest : SysuiTestCase() { @Test fun wifiIcon_isActiveNullLevel_outputsNull() = runBlocking(IMMEDIATE) { + // Even when we should always show the icon + whenever(constants.alwaysShowIconIfEnabled).thenReturn(true) + createAndSetViewModel() + var latest: Icon? = Icon.Resource(0, null) val job = underTest .home @@ -194,9 +231,11 @@ class WifiViewModelTest : SysuiTestCase() { .onEach { latest = it } .launchIn(this) + // WHEN we have a null level wifiRepository.setWifiNetwork(WifiNetworkModel.Active(NETWORK_ID, level = null)) yield() + // THEN we override the alwaysShow boolean and still don't show the icon assertThat(latest).isNull() job.cancel() @@ -233,7 +272,32 @@ class WifiViewModelTest : SysuiTestCase() { } @Test - fun wifiIcon_isActiveAndNotValidated_level4_outputsEmpty4Icon() = runBlocking(IMMEDIATE) { + fun wifiIcon_isActiveAndNotValidated_alwaysShowFalse_outputsNull() = runBlocking(IMMEDIATE) { + whenever(constants.alwaysShowIconIfEnabled).thenReturn(false) + createAndSetViewModel() + + var latest: Icon? = Icon.Resource(0, null) + val job = underTest + .home + .wifiIcon + .onEach { latest = it } + .launchIn(this) + + wifiRepository.setWifiNetwork( + WifiNetworkModel.Active(NETWORK_ID, isValidated = false, level = 4,) + ) + yield() + + assertThat(latest).isNull() + + job.cancel() + } + + @Test + fun wifiIcon_isActiveAndNotValidated_alwaysShowTrue_outputsIcon() = runBlocking(IMMEDIATE) { + whenever(constants.alwaysShowIconIfEnabled).thenReturn(true) + createAndSetViewModel() + var latest: Icon? = null val job = underTest .home