Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b7f10f98 authored by Evan Laird's avatar Evan Laird
Browse files

[sat] Always show icon if Connected or On 

Currently the behavior of the device-based satelilte icon only relied on
all connections being OOS. There are 2 main issues with this:

1. If the radio entered the On or Connected states, we would still wait
   the 10s hysteresis time before showing the connected icon
2. If, for any reason, we got a Connected state when the OOS condition
   was not met, we would fail to show the icon

This change rearranges the logic a bit so that we have the same
provisioned / allowed check, and breaks out the hysteresis visibility
state from the connected/on visibility state. The flows are overall a
bit simpler.

Also added more table logs so we can track more clearly what is going
on.

Bug: 359356088
Flag: NONE bugfix
Test: DeviceBasedSatelliteViewModelTest
Test: manual using satellite demo mode
Change-Id: I4a0d94a29d713b050bab96e8322c577361d3668f
parent 82df6d16

packages/SystemUI/src/com/android/systemui/statusbar/pipeline/satellite/ui/viewmodel/DeviceBasedSatelliteViewModel.kt

+52 −42
Original line number Diff line number Diff line
@@ -36,14 +36,12 @@ import kotlin.time.Duration.Companion.seconds 
import kotlinx.coroutines.CoroutineScope

import kotlinx.coroutines.ExperimentalCoroutinesApi

import kotlinx.coroutines.delay

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.flatMapLatest

import kotlinx.coroutines.flow.flowOf

import kotlinx.coroutines.flow.onEach

import kotlinx.coroutines.flow.stateIn



/**
@@ -76,37 +74,10 @@ constructor( 
    @DeviceBasedSatelliteInputLog logBuffer: LogBuffer,

    @DeviceBasedSatelliteTableLog tableLog: TableLogBuffer,

) : DeviceBasedSatelliteViewModel {

    private val shouldShowIcon: Flow<Boolean> =

        interactor.areAllConnectionsOutOfService

            .flatMapLatest { allOos ->

                if (!allOos) {

                    flowOf(false)

                } else {

                    combine(

                        interactor.isSatelliteAllowed,

                        interactor.isSatelliteProvisioned,

                        interactor.isWifiActive,

                        airplaneModeRepository.isAirplaneMode

                    ) { isSatelliteAllowed, isSatelliteProvisioned, isWifiActive, isAirplaneMode ->

                        isSatelliteAllowed &&

                            isSatelliteProvisioned &&

                            !isWifiActive &&

                            !isAirplaneMode

                    }

                }

            }

            .distinctUntilChanged()

            .logDiffsForTable(

                tableLog,

                columnPrefix = "vm",

                columnName = COL_VISIBLE_CONDITION,

                initialValue = false,

            )



    // This adds a 10 seconds delay before showing the icon

    private val shouldActuallyShowIcon: StateFlow<Boolean> =

        shouldShowIcon

            .distinctUntilChanged()

    private val shouldShowIconForOosAfterHysteresis: StateFlow<Boolean> =

        interactor.areAllConnectionsOutOfService

            .flatMapLatest { shouldShow ->

                if (shouldShow) {

                    logBuffer.log(
@@ -122,6 +93,45 @@ constructor( 
                }

            }

            .distinctUntilChanged()

            .logDiffsForTable(

                tableLog,

                columnPrefix = "vm",

                columnName = COL_VISIBLE_FOR_OOS,

                initialValue = false,

            )

            .stateIn(scope, SharingStarted.WhileSubscribed(), false)



    private val canShowIcon =

        combine(

            interactor.isSatelliteAllowed,

            interactor.isSatelliteProvisioned,

        ) { allowed, provisioned ->

            allowed && provisioned

        }



    private val showIcon =

        canShowIcon

            .flatMapLatest { canShow ->

                if (!canShow) {

                    flowOf(false)

                } else {

                    combine(

                        shouldShowIconForOosAfterHysteresis,

                        interactor.connectionState,

                        interactor.isWifiActive,

                        airplaneModeRepository.isAirplaneMode,

                    ) { showForOos, connectionState, isWifiActive, isAirplaneMode ->

                        if (isWifiActive || isAirplaneMode) {

                            false

                        } else {

                            showForOos ||

                                connectionState == SatelliteConnectionState.On ||

                                connectionState == SatelliteConnectionState.Connected

                        }

                    }

                }

            }

            .distinctUntilChanged()

            .logDiffsForTable(

                tableLog,

                columnPrefix = "vm",
@@ -132,7 +142,7 @@ constructor( 


    override val icon: StateFlow<Icon?> =

        combine(

                shouldActuallyShowIcon,

                showIcon,

                interactor.connectionState,

                interactor.signalStrength,

            ) { shouldShow, state, signalStrength ->
@@ -146,7 +156,7 @@ constructor( 


    override val carrierText: StateFlow<String?> =

        combine(

                shouldActuallyShowIcon,

                showIcon,

                interactor.connectionState,

            ) { shouldShow, connectionState ->

                logBuffer.log(
@@ -172,21 +182,21 @@ constructor( 
                    null

                }

            }

            .onEach {

                logBuffer.log(

                    TAG,

                    LogLevel.INFO,

                    { str1 = it },

                    { "Resulting carrier text = $str1" }

            .distinctUntilChanged()

            .logDiffsForTable(

                tableLog,

                columnPrefix = "vm",

                columnName = COL_CARRIER_TEXT,

                initialValue = null,

            )

            }

            .stateIn(scope, SharingStarted.WhileSubscribed(), null)



    companion object {

        private const val TAG = "DeviceBasedSatelliteViewModel"

        private val DELAY_DURATION = 10.seconds



        const val COL_VISIBLE_CONDITION = "visCondition"

        const val COL_VISIBLE_FOR_OOS = "visibleForOos"

        const val COL_VISIBLE = "visible"

        const val COL_CARRIER_TEXT = "carrierText"

    }

}

packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/ui/viewmodel/DeviceBasedSatelliteViewModelTest.kt

+108 −50
Original line number Diff line number Diff line
@@ -45,6 +45,7 @@ import org.junit.runner.RunWith 
import org.mockito.MockitoAnnotations

import org.mockito.kotlin.mock



@OptIn(ExperimentalCoroutinesApi::class)

@SmallTest

@RunWith(AndroidJUnit4::class)

class DeviceBasedSatelliteViewModelTest : SysuiTestCase() {
@@ -88,7 +89,7 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
    }



    @Test

    fun icon_nullWhenShouldNotShow_satelliteNotAllowed() =

    fun icon_null_satelliteNotAllowed() =

        testScope.runTest {

            val latest by collectLastValue(underTest.icon)
@@ -108,7 +109,30 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
        }



    @Test

    fun icon_nullWhenShouldNotShow_notAllOos() =

    fun icon_null_connectedAndNotAllowed() =

        testScope.runTest {

            val latest by collectLastValue(underTest.icon)



            // GIVEN satellite is not allowed

            repo.isSatelliteAllowedForCurrentLocation.value = false



            // GIVEN all icons are OOS

            val i1 = mobileIconsInteractor.getMobileConnectionInteractorForSubId(1)

            i1.isInService.value = false

            i1.isEmergencyOnly.value = false



            // GIVEN satellite state is Connected. (this should not ever occur, but still)

            repo.connectionState.value = SatelliteConnectionState.Connected



            // GIVEN apm is disabled

            airplaneModeRepository.setIsAirplaneMode(false)



            // THEN icon is null despite the connected state

            assertThat(latest).isNull()

        }



    @Test

    fun icon_null_notAllOos() =

        testScope.runTest {

            val latest by collectLastValue(underTest.icon)
@@ -127,9 +151,28 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            assertThat(latest).isNull()

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun icon_nullWhenShouldNotShow_isEmergencyOnly() =

    fun icon_null_allOosAndNotAllowed() =

        testScope.runTest {

            val latest by collectLastValue(underTest.icon)



            // GIVEN satellite is allowed

            repo.isSatelliteAllowedForCurrentLocation.value = false



            // GIVEN all icons are OOS

            val i1 = mobileIconsInteractor.getMobileConnectionInteractorForSubId(1)

            i1.isInService.value = false

            i1.isEmergencyOnly.value = false



            // GIVEN apm is disabled

            airplaneModeRepository.setIsAirplaneMode(false)



            // THEN icon is null because it is not allowed

            assertThat(latest).isNull()

        }



    @Test

    fun icon_null_isEmergencyOnly() =

        testScope.runTest {

            val latest by collectLastValue(underTest.icon)
@@ -158,7 +201,7 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
        }



    @Test

    fun icon_nullWhenShouldNotShow_apmIsEnabled() =

    fun icon_null_apmIsEnabled() =

        testScope.runTest {

            val latest by collectLastValue(underTest.icon)
@@ -177,9 +220,8 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            assertThat(latest).isNull()

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun icon_satelliteIsOn() =

    fun icon_notNull_satelliteAllowedAndAllOos() =

        testScope.runTest {

            val latest by collectLastValue(underTest.icon)
@@ -201,7 +243,6 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            assertThat(latest).isInstanceOf(Icon::class.java)

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun icon_hysteresisWhenEnablingIcon() =

        testScope.runTest {
@@ -234,9 +275,56 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            assertThat(latest).isNull()

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun icon_deviceIsProvisioned() =

    fun icon_ignoresHysteresis_whenConnected() =

        testScope.runTest {

            val latest by collectLastValue(underTest.icon)



            // GIVEN satellite is allowed

            repo.isSatelliteAllowedForCurrentLocation.value = true



            // GIVEN all icons are OOS

            val i1 = mobileIconsInteractor.getMobileConnectionInteractorForSubId(1)

            i1.isInService.value = false

            i1.isEmergencyOnly.value = false



            // GIVEN apm is disabled

            airplaneModeRepository.setIsAirplaneMode(false)



            // GIVEN satellite reports that it is Connected

            repo.connectionState.value = SatelliteConnectionState.Connected



            // THEN icon is non null because we are connected, despite the normal OOS icon waiting

            // 10 seconds for hysteresis

            assertThat(latest).isInstanceOf(Icon::class.java)

        }



    @Test

    fun icon_ignoresHysteresis_whenOn() =

        testScope.runTest {

            val latest by collectLastValue(underTest.icon)



            // GIVEN satellite is allowed

            repo.isSatelliteAllowedForCurrentLocation.value = true



            // GIVEN all icons are OOS

            val i1 = mobileIconsInteractor.getMobileConnectionInteractorForSubId(1)

            i1.isInService.value = false

            i1.isEmergencyOnly.value = false



            // GIVEN apm is disabled

            airplaneModeRepository.setIsAirplaneMode(false)



            // GIVEN satellite reports that it is Connected

            repo.connectionState.value = SatelliteConnectionState.On



            // THEN icon is non null because the connection state is On, despite the normal OOS icon

            // waiting 10 seconds for hysteresis

            assertThat(latest).isInstanceOf(Icon::class.java)

        }



    @Test

    fun icon_satelliteIsProvisioned() =

        testScope.runTest {

            val latest by collectLastValue(underTest.icon)
@@ -267,7 +355,6 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            assertThat(latest).isInstanceOf(Icon::class.java)

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun icon_wifiIsActive() =

        testScope.runTest {
@@ -324,13 +411,13 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
        }



    @Test

    fun carrierText_nullWhenShouldNotShow_notAllOos() =

    fun carrierText_null_notAllOos() =

        testScope.runTest {

            val latest by collectLastValue(underTest.carrierText)



            // GIVEN satellite is allowed + connected

            // GIVEN satellite is allowed + off

            repo.isSatelliteAllowedForCurrentLocation.value = true

            repo.connectionState.value = SatelliteConnectionState.Connected

            repo.connectionState.value = SatelliteConnectionState.Off



            // GIVEN all icons are not OOS

            val i1 = mobileIconsInteractor.getMobileConnectionInteractorForSubId(1)
@@ -344,9 +431,8 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            assertThat(latest).isNull()

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun carrierText_nullWhenShouldNotShow_isEmergencyOnly() =

    fun carrierText_notNull_notAllOos_butConnected() =

        testScope.runTest {

            val latest by collectLastValue(underTest.carrierText)
@@ -354,25 +440,17 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            repo.isSatelliteAllowedForCurrentLocation.value = true

            repo.connectionState.value = SatelliteConnectionState.Connected



            // GIVEN all icons are OOS

            // GIVEN all icons are not OOS

            val i1 = mobileIconsInteractor.getMobileConnectionInteractorForSubId(1)

            i1.isInService.value = false

            i1.isInService.value = true

            i1.isEmergencyOnly.value = false



            // GIVEN apm is disabled

            airplaneModeRepository.setIsAirplaneMode(false)



            // Wait for delay to be completed

            advanceTimeBy(10.seconds)



            // THEN carrier text is set because we don't have service

            // THEN carrier text is not null, because it is connected

            // This case should never happen, but let's test it anyway

            assertThat(latest).isNotNull()



            // GIVEN the connection is emergency only

            i1.isEmergencyOnly.value = true



            // THEN carrier text is null because we have emergency connection

            assertThat(latest).isNull()

        }



    @Test
@@ -396,7 +474,6 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            assertThat(latest).isNull()

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun carrierText_satelliteIsOn() =

        testScope.runTest {
@@ -421,9 +498,8 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            assertThat(latest).isNotNull()

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun carrierText_hysteresisWhenEnablingText() =

    fun carrierText_noHysteresisWhenEnablingText_connected() =

        testScope.runTest {

            val latest by collectLastValue(underTest.carrierText)
@@ -439,23 +515,10 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            // GIVEN apm is disabled

            airplaneModeRepository.setIsAirplaneMode(false)



            // THEN carrier text is null because of the hysteresis

            assertThat(latest).isNull()



            // Wait for delay to be completed

            advanceTimeBy(10.seconds)



            // THEN carrier text is set after the delay

            // THEN carrier text is not null because we skip hysteresis when connected

            assertThat(latest).isNotNull()



            // GIVEN apm is enabled

            airplaneModeRepository.setIsAirplaneMode(true)



            // THEN carrier text is null immediately

            assertThat(latest).isNull()

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun carrierText_deviceIsProvisioned() =

        testScope.runTest {
@@ -489,7 +552,6 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            assertThat(latest).isNotNull()

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun carrierText_wifiIsActive() =

        testScope.runTest {
@@ -526,7 +588,6 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            assertThat(latest).isNotNull()

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun carrierText_connectionStateUnknown_null() =

        testScope.runTest {
@@ -547,7 +608,6 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            assertThat(latest).isNull()

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun carrierText_connectionStateOff_null() =

        testScope.runTest {
@@ -568,7 +628,6 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
            assertThat(latest).isNull()

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun carrierText_connectionStateOn_notConnectedString() =

        testScope.runTest {
@@ -590,7 +649,6 @@ class DeviceBasedSatelliteViewModelTest : SysuiTestCase() { 
                .isEqualTo(context.getString(R.string.satellite_connected_carrier_text))

        }



    @OptIn(ExperimentalCoroutinesApi::class)

    @Test

    fun carrierText_connectionStateConnected_connectedString() =

        testScope.runTest {