Merge "[SB Refactor] Always show the wifi icon if the device has no data capabilities." into tm-qpr-dev

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/shared/ConnectivityConstants.kt

@@ -0,0 +1,46 @@
+/*
+ * 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.shared
+
+import android.telephony.TelephonyManager
+import com.android.systemui.Dumpable
+import com.android.systemui.dagger.SysUISingleton
+import com.android.systemui.dump.DumpManager
+import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.SB_LOGGING_TAG
+import java.io.PrintWriter
+import javax.inject.Inject
+
+/**
+ * An object storing constants that are used for calculating connectivity icons.
+ *
+ * Stored in a class for logging purposes.
+ */
+@SysUISingleton
+class ConnectivityConstants
+@Inject
+constructor(dumpManager: DumpManager, telephonyManager: TelephonyManager) : Dumpable {
+    init {
+        dumpManager.registerDumpable("$SB_LOGGING_TAG:ConnectivityConstants", this)
+    }
+
+    /** True if this device has the capability for data connections and false otherwise. */
+    val hasDataCapabilities = telephonyManager.isDataCapable
+
+    override fun dump(pw: PrintWriter, args: Array<out String>) {
+        pw.apply { println("hasDataCapabilities=$hasDataCapabilities") }
+    }
+}

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModel.kt

@@ -31,6 +31,7 @@ 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
 import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags
+import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger.Companion.logOutputChange
 import com.android.systemui.statusbar.pipeline.wifi.data.model.WifiNetworkModel
@@ -65,12 +66,13 @@ import kotlinx.coroutines.flow.stateIn
 class WifiViewModel
 @Inject
 constructor(
-    constants: WifiConstants,
+    connectivityConstants: ConnectivityConstants,
     private val context: Context,
     logger: ConnectivityPipelineLogger,
     interactor: WifiInteractor,
     @Application private val scope: CoroutineScope,
     statusBarPipelineFlags: StatusBarPipelineFlags,
+    wifiConstants: WifiConstants,
 ) {
     /**
      * Returns the drawable resource ID to use for the wifi icon based on the given network.
@@ -133,7 +135,8 @@ constructor(
             val icon = Icon.Resource(iconResId, wifiNetwork.contentDescription())
 
             return@combine when {
-                constants.alwaysShowIconIfEnabled -> icon
+                wifiConstants.alwaysShowIconIfEnabled -> icon
+                !connectivityConstants.hasDataCapabilities -> icon
                 wifiNetwork is WifiNetworkModel.Active && wifiNetwork.isValidated -> icon
                 else -> null
             }
@@ -142,7 +145,7 @@ constructor(
 
     /** The wifi activity status. Null if we shouldn't display the activity status. */
     private val activity: Flow<WifiActivityModel?> =
-        if (!constants.shouldShowActivityConfig) {
+        if (!wifiConstants.shouldShowActivityConfig) {
             flowOf(null)
         } else {
             combine(interactor.activity, interactor.ssid) { activity, ssid ->

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/view/ModernStatusBarWifiViewTest.kt

@@ -30,6 +30,7 @@ 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.StatusBarPipelineFlags
+import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants
 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
@@ -59,7 +60,9 @@ class ModernStatusBarWifiViewTest : SysuiTestCase() {
     @Mock
     private lateinit var logger: ConnectivityPipelineLogger
     @Mock
-    private lateinit var constants: WifiConstants
+    private lateinit var connectivityConstants: ConnectivityConstants
+    @Mock
+    private lateinit var wifiConstants: WifiConstants
     private lateinit var connectivityRepository: FakeConnectivityRepository
     private lateinit var wifiRepository: FakeWifiRepository
     private lateinit var interactor: WifiInteractor
@@ -80,7 +83,13 @@ class ModernStatusBarWifiViewTest : SysuiTestCase() {
         interactor = WifiInteractor(connectivityRepository, wifiRepository)
         scope = CoroutineScope(Dispatchers.Unconfined)
         viewModel = WifiViewModel(
-            constants, context, logger, interactor, scope, statusBarPipelineFlags
+            connectivityConstants,
+            context,
+            logger,
+            interactor,
+            scope,
+            statusBarPipelineFlags,
+            wifiConstants,
         )
     }
 

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/wifi/ui/viewmodel/WifiViewModelTest.kt

@@ -26,6 +26,7 @@ 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
 import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags
+import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger
 import com.android.systemui.statusbar.pipeline.shared.data.model.ConnectivitySlot
 import com.android.systemui.statusbar.pipeline.shared.data.repository.FakeConnectivityRepository
@@ -60,7 +61,8 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Mock private lateinit var statusBarPipelineFlags: StatusBarPipelineFlags
     @Mock private lateinit var logger: ConnectivityPipelineLogger
-    @Mock private lateinit var constants: WifiConstants
+    @Mock private lateinit var connectivityConstants: ConnectivityConstants
+    @Mock private lateinit var wifiConstants: WifiConstants
     private lateinit var connectivityRepository: FakeConnectivityRepository
     private lateinit var wifiRepository: FakeWifiRepository
     private lateinit var interactor: WifiInteractor
@@ -87,6 +89,9 @@ 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.
 
+    // TODO(b/238425913): We should probably parameterize the wifiIcon tests since there's so many
+    //   different possibilities.
+
     @Test
     fun wifiIcon_notEnabled_outputsNull() = runBlocking(IMMEDIATE) {
         wifiRepository.setIsWifiEnabled(false)
@@ -150,7 +155,8 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun wifiIcon_inactiveNetwork_alwaysShowFalse_outputsNull() = runBlocking(IMMEDIATE) {
-        whenever(constants.alwaysShowIconIfEnabled).thenReturn(false)
+        whenever(wifiConstants.alwaysShowIconIfEnabled).thenReturn(false)
+        whenever(connectivityConstants.hasDataCapabilities).thenReturn(true)
         createAndSetViewModel()
 
         // Start as non-null so we can verify we got the update
@@ -171,7 +177,54 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun wifiIcon_inactiveNetwork_alwaysShowTrue_outputsNoNetworkIcon() = runBlocking(IMMEDIATE) {
-        whenever(constants.alwaysShowIconIfEnabled).thenReturn(true)
+        whenever(wifiConstants.alwaysShowIconIfEnabled).thenReturn(true)
+        createAndSetViewModel()
+
+        var latest: Icon? = null
+        val job = underTest
+            .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
+        assertThat(icon.res).isEqualTo(WIFI_NO_NETWORK)
+        assertThat(icon.contentDescription?.getAsString())
+            .contains(context.getString(WIFI_NO_CONNECTION))
+        assertThat(icon.contentDescription?.getAsString())
+            .contains(context.getString(NO_INTERNET))
+
+        job.cancel()
+    }
+
+    @Test
+    fun wifiIcon_inactiveNetwork_hasDataCaps_outputsNull() = runBlocking(IMMEDIATE) {
+        whenever(connectivityConstants.hasDataCapabilities).thenReturn(true)
+        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_noDataCaps_outputsNoNetworkIcon() = runBlocking(IMMEDIATE) {
+        whenever(connectivityConstants.hasDataCapabilities).thenReturn(false)
         createAndSetViewModel()
 
         var latest: Icon? = null
@@ -198,7 +251,7 @@ class WifiViewModelTest : SysuiTestCase() {
     @Test
     fun wifiIcon_carrierMergedNetwork_outputsNull() = runBlocking(IMMEDIATE) {
         // Even when we should always show the icon
-        whenever(constants.alwaysShowIconIfEnabled).thenReturn(true)
+        whenever(wifiConstants.alwaysShowIconIfEnabled).thenReturn(true)
         createAndSetViewModel()
 
         var latest: Icon? = Icon.Resource(0, null)
@@ -221,7 +274,7 @@ class WifiViewModelTest : SysuiTestCase() {
     @Test
     fun wifiIcon_isActiveNullLevel_outputsNull() = runBlocking(IMMEDIATE) {
         // Even when we should always show the icon
-        whenever(constants.alwaysShowIconIfEnabled).thenReturn(true)
+        whenever(wifiConstants.alwaysShowIconIfEnabled).thenReturn(true)
         createAndSetViewModel()
 
         var latest: Icon? = Icon.Resource(0, null)
@@ -273,7 +326,8 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun wifiIcon_isActiveAndNotValidated_alwaysShowFalse_outputsNull() = runBlocking(IMMEDIATE) {
-        whenever(constants.alwaysShowIconIfEnabled).thenReturn(false)
+        whenever(wifiConstants.alwaysShowIconIfEnabled).thenReturn(false)
+        whenever(connectivityConstants.hasDataCapabilities).thenReturn(true)
         createAndSetViewModel()
 
         var latest: Icon? = Icon.Resource(0, null)
@@ -295,7 +349,62 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun wifiIcon_isActiveAndNotValidated_alwaysShowTrue_outputsIcon() = runBlocking(IMMEDIATE) {
-        whenever(constants.alwaysShowIconIfEnabled).thenReturn(true)
+        whenever(wifiConstants.alwaysShowIconIfEnabled).thenReturn(true)
+        createAndSetViewModel()
+
+        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])
+        assertThat(icon.contentDescription?.getAsString())
+            .contains(context.getString(WIFI_CONNECTION_STRENGTH[level]))
+        assertThat(icon.contentDescription?.getAsString())
+            .contains(context.getString(NO_INTERNET))
+
+        job.cancel()
+    }
+
+    @Test
+    fun wifiIcon_isActiveAndNotValidated_hasDataCaps_outputsNull() = runBlocking(IMMEDIATE) {
+        whenever(connectivityConstants.hasDataCapabilities).thenReturn(true)
+        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_noDataCaps_outputsIcon() = runBlocking(IMMEDIATE) {
+        whenever(connectivityConstants.hasDataCapabilities).thenReturn(false)
         createAndSetViewModel()
 
         var latest: Icon? = null
@@ -369,7 +478,7 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun activity_showActivityConfigFalse_outputsFalse() = runBlocking(IMMEDIATE) {
-        whenever(constants.shouldShowActivityConfig).thenReturn(false)
+        whenever(wifiConstants.shouldShowActivityConfig).thenReturn(false)
         createAndSetViewModel()
         wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK)
 
@@ -406,7 +515,7 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun activity_showActivityConfigFalse_noUpdatesReceived() = runBlocking(IMMEDIATE) {
-        whenever(constants.shouldShowActivityConfig).thenReturn(false)
+        whenever(wifiConstants.shouldShowActivityConfig).thenReturn(false)
         createAndSetViewModel()
         wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK)
 
@@ -448,7 +557,7 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun activity_nullSsid_outputsFalse() = runBlocking(IMMEDIATE) {
-        whenever(constants.shouldShowActivityConfig).thenReturn(true)
+        whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true)
         createAndSetViewModel()
 
         wifiRepository.setWifiNetwork(WifiNetworkModel.Active(NETWORK_ID, ssid = null))
@@ -491,7 +600,7 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun activity_allLocationViewModelsReceiveSameData() = runBlocking(IMMEDIATE) {
-        whenever(constants.shouldShowActivityConfig).thenReturn(true)
+        whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true)
         createAndSetViewModel()
         wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK)
 
@@ -531,7 +640,7 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun activityIn_hasActivityInTrue_outputsTrue() = runBlocking(IMMEDIATE) {
-        whenever(constants.shouldShowActivityConfig).thenReturn(true)
+        whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true)
         createAndSetViewModel()
         wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK)
 
@@ -553,7 +662,7 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun activityIn_hasActivityInFalse_outputsFalse() = runBlocking(IMMEDIATE) {
-        whenever(constants.shouldShowActivityConfig).thenReturn(true)
+        whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true)
         createAndSetViewModel()
         wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK)
 
@@ -575,7 +684,7 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun activityOut_hasActivityOutTrue_outputsTrue() = runBlocking(IMMEDIATE) {
-        whenever(constants.shouldShowActivityConfig).thenReturn(true)
+        whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true)
         createAndSetViewModel()
         wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK)
 
@@ -597,7 +706,7 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun activityOut_hasActivityOutFalse_outputsFalse() = runBlocking(IMMEDIATE) {
-        whenever(constants.shouldShowActivityConfig).thenReturn(true)
+        whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true)
         createAndSetViewModel()
         wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK)
 
@@ -619,7 +728,7 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun activityContainer_hasActivityInTrue_outputsTrue() = runBlocking(IMMEDIATE) {
-        whenever(constants.shouldShowActivityConfig).thenReturn(true)
+        whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true)
         createAndSetViewModel()
         wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK)
 
@@ -641,7 +750,7 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun activityContainer_hasActivityOutTrue_outputsTrue() = runBlocking(IMMEDIATE) {
-        whenever(constants.shouldShowActivityConfig).thenReturn(true)
+        whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true)
         createAndSetViewModel()
         wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK)
 
@@ -663,7 +772,7 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun activityContainer_inAndOutTrue_outputsTrue() = runBlocking(IMMEDIATE) {
-        whenever(constants.shouldShowActivityConfig).thenReturn(true)
+        whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true)
         createAndSetViewModel()
         wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK)
 
@@ -685,7 +794,7 @@ class WifiViewModelTest : SysuiTestCase() {
 
     @Test
     fun activityContainer_inAndOutFalse_outputsFalse() = runBlocking(IMMEDIATE) {
-        whenever(constants.shouldShowActivityConfig).thenReturn(true)
+        whenever(wifiConstants.shouldShowActivityConfig).thenReturn(true)
         createAndSetViewModel()
         wifiRepository.setWifiNetwork(ACTIVE_VALID_WIFI_NETWORK)
 
@@ -710,12 +819,13 @@ class WifiViewModelTest : SysuiTestCase() {
         // 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,
+            connectivityConstants,
             context,
             logger,
             interactor,
             scope,
             statusBarPipelineFlags,
+            wifiConstants,
         )
     }