From 0a7c73ada072af6a650530987c5e7f73d0237390 Mon Sep 17 00:00:00 2001 From: Caitlin Shkuratov Date: Fri, 14 Apr 2023 14:11:35 +0000 Subject: [PATCH] [SB Refactor] In the mobile view model, move `isInService` calculation. While looking into b/266625531, I almost missed the fact that `isInService` is used to determine whether or not to show the ! because the `isInService` wasn't part of the `showExclamationMark` flow. And, that means that our `showExclamationMark` logs might be wrong, because `isInService=false` will cause us to show the ! but our logs would say `showExclamationMark=false` incorrectly. This CL: 1) Moves `isInService` to be part of the `showExclamationMark` flow instead of the `icon` flow. 2) Creates a new `shownLevel` flow, which then simplifies the `icon` and `contentDescription` flows. Bug: 238425913 Test: atest MobileIconViewModelTest (existing tests already cover `isInService` affecting ! and level) Change-Id: Ibfc64cac91e1df537cc2b44635232cd0f848be56 --- .../mobile/ui/model/SignalIconModel.kt | 4 -- .../ui/viewmodel/MobileIconViewModel.kt | 58 +++++++++---------- 2 files changed, 29 insertions(+), 33 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/model/SignalIconModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/model/SignalIconModel.kt index 16e176613ec94..be2e41a2a5817 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/model/SignalIconModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/model/SignalIconModel.kt @@ -45,10 +45,6 @@ data class SignalIconModel( } 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" diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt index bfd133e6830c9..54730ed825643 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt @@ -17,7 +17,6 @@ package com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel 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.systemui.common.shared.model.ContentDescription import com.android.systemui.common.shared.model.Icon @@ -78,13 +77,24 @@ constructor( scope: CoroutineScope, ) : MobileIconViewModelCommon { /** Whether or not to show the error state of [SignalDrawable] */ - private val showExclamationMark: Flow = + private val showExclamationMark: StateFlow = combine( - iconInteractor.isDefaultDataEnabled, - iconInteractor.isDefaultConnectionFailed, - ) { isDefaultDataEnabled, isDefaultConnectionFailed -> - !isDefaultDataEnabled || isDefaultConnectionFailed - } + iconInteractor.isDefaultDataEnabled, + iconInteractor.isDefaultConnectionFailed, + iconInteractor.isInService, + ) { isDefaultDataEnabled, isDefaultConnectionFailed, isInService -> + !isDefaultDataEnabled || isDefaultConnectionFailed || !isInService + } + .stateIn(scope, SharingStarted.WhileSubscribed(), true) + + private val shownLevel: StateFlow = + combine( + iconInteractor.level, + iconInteractor.isInService, + ) { level, isInService -> + if (isInService) level else 0 + } + .stateIn(scope, SharingStarted.WhileSubscribed(), 0) override val isVisible: StateFlow = if (!constants.hasDataCapabilities) { @@ -107,18 +117,18 @@ constructor( .stateIn(scope, SharingStarted.WhileSubscribed(), false) override val icon: Flow = run { - val initial = SignalIconModel.createEmptyState(iconInteractor.numberOfLevels.value) + val initial = + SignalIconModel( + level = shownLevel.value, + numberOfLevels = iconInteractor.numberOfLevels.value, + showExclamationMark = showExclamationMark.value, + ) combine( - iconInteractor.level, + shownLevel, iconInteractor.numberOfLevels, showExclamationMark, - iconInteractor.isInService, - ) { level, numberOfLevels, showExclamationMark, isInService -> - if (!isInService) { - SignalIconModel.createEmptyState(numberOfLevels) - } else { - SignalIconModel(level, numberOfLevels, showExclamationMark) - } + ) { shownLevel, numberOfLevels, showExclamationMark -> + SignalIconModel(shownLevel, numberOfLevels, showExclamationMark) } .distinctUntilChanged() .logDiffsForTable( @@ -130,19 +140,9 @@ constructor( } override val contentDescription: Flow = run { - val initial = ContentDescription.Resource(PHONE_SIGNAL_STRENGTH_NONE) - combine( - iconInteractor.level, - iconInteractor.isInService, - ) { level, isInService -> - val resId = - when { - isInService -> PHONE_SIGNAL_STRENGTH[level] - else -> PHONE_SIGNAL_STRENGTH_NONE - } - ContentDescription.Resource(resId) - } - .distinctUntilChanged() + val initial = ContentDescription.Resource(PHONE_SIGNAL_STRENGTH[0]) + shownLevel + .map { ContentDescription.Resource(PHONE_SIGNAL_STRENGTH[it]) } .stateIn(scope, SharingStarted.WhileSubscribed(), initial) }