[SB Refactor] Add better logging for the mobile icon ID.

Previous logging had `iconId=1028` or `iconId=132096`, which is
difficult to parse.

Bug: 238425913
Test: Manual: Dumped DemoMobileConnectionLog [1] and verified output
Test: atest MobileIconViewModelTest
Change-Id: I8ddeafe4dfb579d9d05e5c23a62bf2e55e256998

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/binder/MobileIconBinder.kt

@@ -93,8 +93,13 @@ object MobileIconBinder {
 
                 // Set the icon for the triangle
                 launch {
-                    viewModel.iconId.distinctUntilChanged().collect { iconId ->
-                        mobileDrawable.level = iconId
+                    viewModel.icon.distinctUntilChanged().collect { icon ->
+                        mobileDrawable.level =
+                            SignalDrawable.getState(
+                                icon.level,
+                                icon.numberOfLevels,
+                                icon.showExclamationMark,
+                            )
                     }
                 }
 

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/model/SignalIconModel.kt

@@ -0,0 +1,56 @@
+/*
+ * Copyright (C) 2023 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.systemui.statusbar.pipeline.mobile.ui.model
+
+import com.android.systemui.log.table.Diffable
+import com.android.systemui.log.table.TableRowLogger
+
+/** A model that will be consumed by [SignalDrawable] to show the mobile triangle icon. */
+data class SignalIconModel(
+    val level: Int,
+    val numberOfLevels: Int,
+    val showExclamationMark: Boolean,
+) : Diffable<SignalIconModel> {
+    // TODO(b/267767715): Can we implement [logDiffs] and [logFull] generically for data classes?
+    override fun logDiffs(prevVal: SignalIconModel, row: TableRowLogger) {
+        if (prevVal.level != level) {
+            row.logChange(COL_LEVEL, level)
+        }
+        if (prevVal.numberOfLevels != numberOfLevels) {
+            row.logChange(COL_NUM_LEVELS, numberOfLevels)
+        }
+        if (prevVal.showExclamationMark != showExclamationMark) {
+            row.logChange(COL_SHOW_EXCLAMATION, showExclamationMark)
+        }
+    }
+
+    override fun logFull(row: TableRowLogger) {
+        row.logChange(COL_LEVEL, level)
+        row.logChange(COL_NUM_LEVELS, numberOfLevels)
+        row.logChange(COL_SHOW_EXCLAMATION, showExclamationMark)
+    }
+
+    companion object {
+        /** Creates a [SignalIconModel] representing an empty and invalidated state. */
+        fun createEmptyState(numberOfLevels: Int) =
+            SignalIconModel(level = 0, numberOfLevels, showExclamationMark = true)
+
+        private const val COL_LEVEL = "level"
+        private const val COL_NUM_LEVELS = "numLevels"
+        private const val COL_SHOW_EXCLAMATION = "showExclamation"
+    }
+}

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt

@@ -24,8 +24,8 @@ import com.android.systemui.common.shared.model.Icon
 import com.android.systemui.log.table.logDiffsForTable
 import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.MobileIconInteractor
 import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.MobileIconsInteractor
+import com.android.systemui.statusbar.pipeline.mobile.ui.model.SignalIconModel
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants
-import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger
 import com.android.systemui.statusbar.pipeline.shared.data.model.DataActivityModel
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -42,8 +42,7 @@ import kotlinx.coroutines.flow.stateIn
 /** Common interface for all of the location-based mobile icon view models. */
 interface MobileIconViewModelCommon {
     val subscriptionId: Int
-    /** An int consumable by [SignalDrawable] for display */
-    val iconId: Flow<Int>
+    val icon: Flow<SignalIconModel>
     val contentDescription: Flow<ContentDescription>
     val roaming: Flow<Boolean>
     /** The RAT icon (LTE, 3G, 5G, etc) to be displayed. Null if we shouldn't show anything */
@@ -72,7 +71,6 @@ class MobileIconViewModel
 constructor(
     override val subscriptionId: Int,
     iconInteractor: MobileIconInteractor,
-    logger: ConnectivityPipelineLogger,
     constants: ConnectivityConstants,
     scope: CoroutineScope,
 ) : MobileIconViewModelCommon {
@@ -80,8 +78,8 @@ constructor(
     private val showExclamationMark: Flow<Boolean> =
         iconInteractor.isDefaultDataEnabled.mapLatest { !it }
 
-    override val iconId: Flow<Int> = run {
-        val initial = SignalDrawable.getEmptyState(iconInteractor.numberOfLevels.value)
+    override val icon: Flow<SignalIconModel> = run {
+        val initial = SignalIconModel.createEmptyState(iconInteractor.numberOfLevels.value)
         combine(
                 iconInteractor.level,
                 iconInteractor.numberOfLevels,
@@ -89,16 +87,15 @@ constructor(
                 iconInteractor.isInService,
             ) { level, numberOfLevels, showExclamationMark, isInService ->
                 if (!isInService) {
-                    SignalDrawable.getEmptyState(numberOfLevels)
+                    SignalIconModel.createEmptyState(numberOfLevels)
                 } else {
-                    SignalDrawable.getState(level, numberOfLevels, showExclamationMark)
+                    SignalIconModel(level, numberOfLevels, showExclamationMark)
                 }
             }
             .distinctUntilChanged()
             .logDiffsForTable(
                 iconInteractor.tableLogBuffer,
-                columnPrefix = "",
-                columnName = "iconId",
+                columnPrefix = "icon",
                 initialValue = initial,
             )
             .stateIn(scope, SharingStarted.WhileSubscribed(), initial)

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconsViewModel.kt

@@ -56,7 +56,6 @@ constructor(
                 ?: MobileIconViewModel(
                         subId,
                         interactor.createMobileConnectionInteractorForSubId(subId),
-                        logger,
                         constants,
                         scope,
                     )

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/view/ModernStatusBarMobileViewTest.kt

@@ -34,7 +34,6 @@ import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.LocationBased
 import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.MobileIconViewModel
 import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.QsMobileIconViewModel
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants
-import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger
 import com.android.systemui.util.mockito.whenever
 import com.google.common.truth.Truth.assertThat
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -58,7 +57,6 @@ class ModernStatusBarMobileViewTest : SysuiTestCase() {
 
     @Mock private lateinit var statusBarPipelineFlags: StatusBarPipelineFlags
     @Mock private lateinit var tableLogBuffer: TableLogBuffer
-    @Mock private lateinit var logger: ConnectivityPipelineLogger
     @Mock private lateinit var constants: ConnectivityConstants
 
     private lateinit var viewModel: LocationBasedMobileViewModel
@@ -74,7 +72,6 @@ class ModernStatusBarMobileViewTest : SysuiTestCase() {
             MobileIconViewModel(
                 subscriptionId = 1,
                 interactor,
-                logger,
                 constants,
                 testScope.backgroundScope,
             )

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/LocationBasedMobileIconViewModelTest.kt

@@ -22,9 +22,9 @@ import com.android.systemui.SysuiTestCase
 import com.android.systemui.log.table.TableLogBuffer
 import com.android.systemui.statusbar.pipeline.StatusBarPipelineFlags
 import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.FakeMobileIconInteractor
+import com.android.systemui.statusbar.pipeline.mobile.ui.model.SignalIconModel
 import com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel.MobileIconViewModelTest.Companion.defaultSignal
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants
-import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger
 import com.google.common.truth.Truth.assertThat
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.flow.launchIn
@@ -47,7 +47,6 @@ class LocationBasedMobileIconViewModelTest : SysuiTestCase() {
     private lateinit var keyguardIcon: KeyguardMobileIconViewModel
     private lateinit var interactor: FakeMobileIconInteractor
     @Mock private lateinit var statusBarPipelineFlags: StatusBarPipelineFlags
-    @Mock private lateinit var logger: ConnectivityPipelineLogger
     @Mock private lateinit var constants: ConnectivityConstants
     @Mock private lateinit var tableLogBuffer: TableLogBuffer
 
@@ -67,8 +66,7 @@ class LocationBasedMobileIconViewModelTest : SysuiTestCase() {
             setNumberOfLevels(4)
             isDataConnected.value = true
         }
-        commonImpl =
-            MobileIconViewModel(SUB_1_ID, interactor, logger, constants, testScope.backgroundScope)
+        commonImpl = MobileIconViewModel(SUB_1_ID, interactor, constants, testScope.backgroundScope)
 
         homeIcon = HomeMobileIconViewModel(commonImpl, statusBarPipelineFlags)
         qsIcon = QsMobileIconViewModel(commonImpl, statusBarPipelineFlags)
@@ -78,14 +76,14 @@ class LocationBasedMobileIconViewModelTest : SysuiTestCase() {
     @Test
     fun `location based view models receive same icon id when common impl updates`() =
         testScope.runTest {
-            var latestHome: Int? = null
-            val homeJob = homeIcon.iconId.onEach { latestHome = it }.launchIn(this)
+            var latestHome: SignalIconModel? = null
+            val homeJob = homeIcon.icon.onEach { latestHome = it }.launchIn(this)
 
-            var latestQs: Int? = null
-            val qsJob = qsIcon.iconId.onEach { latestQs = it }.launchIn(this)
+            var latestQs: SignalIconModel? = null
+            val qsJob = qsIcon.icon.onEach { latestQs = it }.launchIn(this)
 
-            var latestKeyguard: Int? = null
-            val keyguardJob = keyguardIcon.iconId.onEach { latestKeyguard = it }.launchIn(this)
+            var latestKeyguard: SignalIconModel? = null
+            val keyguardJob = keyguardIcon.icon.onEach { latestKeyguard = it }.launchIn(this)
 
             var expected = defaultSignal(level = 1)
 

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt

@@ -19,15 +19,14 @@ package com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel
 import androidx.test.filters.SmallTest
 import com.android.settingslib.AccessibilityContentDescriptions.PHONE_SIGNAL_STRENGTH
 import com.android.settingslib.AccessibilityContentDescriptions.PHONE_SIGNAL_STRENGTH_NONE
-import com.android.settingslib.graph.SignalDrawable
 import com.android.settingslib.mobile.TelephonyIcons.THREE_G
 import com.android.systemui.SysuiTestCase
 import com.android.systemui.common.shared.model.ContentDescription
 import com.android.systemui.common.shared.model.Icon
 import com.android.systemui.log.table.TableLogBuffer
 import com.android.systemui.statusbar.pipeline.mobile.domain.interactor.FakeMobileIconInteractor
+import com.android.systemui.statusbar.pipeline.mobile.ui.model.SignalIconModel
 import com.android.systemui.statusbar.pipeline.shared.ConnectivityConstants
-import com.android.systemui.statusbar.pipeline.shared.ConnectivityPipelineLogger
 import com.android.systemui.statusbar.pipeline.shared.data.model.DataActivityModel
 import com.android.systemui.util.mockito.whenever
 import com.google.common.truth.Truth.assertThat
@@ -49,7 +48,6 @@ import org.mockito.MockitoAnnotations
 class MobileIconViewModelTest : SysuiTestCase() {
     private lateinit var underTest: MobileIconViewModel
     private lateinit var interactor: FakeMobileIconInteractor
-    @Mock private lateinit var logger: ConnectivityPipelineLogger
     @Mock private lateinit var constants: ConnectivityConstants
     @Mock private lateinit var tableLogBuffer: TableLogBuffer
 
@@ -69,15 +67,14 @@ class MobileIconViewModelTest : SysuiTestCase() {
             setNumberOfLevels(4)
             isDataConnected.value = true
         }
-        underTest =
-            MobileIconViewModel(SUB_1_ID, interactor, logger, constants, testScope.backgroundScope)
+        underTest = MobileIconViewModel(SUB_1_ID, interactor, constants, testScope.backgroundScope)
     }
 
     @Test
     fun iconId_correctLevel_notCutout() =
         testScope.runTest {
-            var latest: Int? = null
-            val job = underTest.iconId.onEach { latest = it }.launchIn(this)
+            var latest: SignalIconModel? = null
+            val job = underTest.icon.onEach { latest = it }.launchIn(this)
             val expected = defaultSignal()
 
             assertThat(latest).isEqualTo(expected)
@@ -90,8 +87,8 @@ class MobileIconViewModelTest : SysuiTestCase() {
         testScope.runTest {
             interactor.setIsDefaultDataEnabled(false)
 
-            var latest: Int? = null
-            val job = underTest.iconId.onEach { latest = it }.launchIn(this)
+            var latest: SignalIconModel? = null
+            val job = underTest.icon.onEach { latest = it }.launchIn(this)
             val expected = defaultSignal(level = 1, connected = false)
 
             assertThat(latest).isEqualTo(expected)
@@ -102,8 +99,8 @@ class MobileIconViewModelTest : SysuiTestCase() {
     @Test
     fun `icon - uses empty state - when not in service`() =
         testScope.runTest {
-            var latest: Int? = null
-            val job = underTest.iconId.onEach { latest = it }.launchIn(this)
+            var latest: SignalIconModel? = null
+            val job = underTest.icon.onEach { latest = it }.launchIn(this)
 
             interactor.isInService.value = false
 
@@ -368,7 +365,6 @@ class MobileIconViewModelTest : SysuiTestCase() {
                 MobileIconViewModel(
                     SUB_1_ID,
                     interactor,
-                    logger,
                     constants,
                     testScope.backgroundScope,
                 )
@@ -407,7 +403,6 @@ class MobileIconViewModelTest : SysuiTestCase() {
                 MobileIconViewModel(
                     SUB_1_ID,
                     interactor,
-                    logger,
                     constants,
                     testScope.backgroundScope,
                 )
@@ -466,10 +461,11 @@ class MobileIconViewModelTest : SysuiTestCase() {
         fun defaultSignal(
             level: Int = 1,
             connected: Boolean = true,
-        ): Int {
-            return SignalDrawable.getState(level, /* numLevels */ 4, !connected)
+        ): SignalIconModel {
+            return SignalIconModel(level, numberOfLevels = 4, showExclamationMark = !connected)
         }
 
-        fun emptySignal(): Int = SignalDrawable.getEmptyState(4)
+        fun emptySignal(): SignalIconModel =
+            SignalIconModel(level = 0, numberOfLevels = 4, showExclamationMark = true)
     }
 }