diff options
2 files changed, 97 insertions, 201 deletions
diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/satellite/data/prod/DeviceBasedSatelliteRepositoryImpl.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/satellite/data/prod/DeviceBasedSatelliteRepositoryImpl.kt index a7c5f78e5b69..03ec41d5af46 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/satellite/data/prod/DeviceBasedSatelliteRepositoryImpl.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/satellite/data/prod/DeviceBasedSatelliteRepositoryImpl.kt @@ -20,6 +20,7 @@ import android.os.OutcomeReceiver import android.telephony.TelephonyCallback import android.telephony.TelephonyManager import android.telephony.satellite.NtnSignalStrengthCallback +import android.telephony.satellite.SatelliteCommunicationAllowedStateCallback import android.telephony.satellite.SatelliteManager import android.telephony.satellite.SatelliteManager.SATELLITE_RESULT_SUCCESS import android.telephony.satellite.SatelliteModemStateCallback @@ -37,7 +38,6 @@ import com.android.systemui.log.core.MessagePrinter import com.android.systemui.statusbar.pipeline.dagger.DeviceBasedSatelliteInputLog import com.android.systemui.statusbar.pipeline.dagger.VerboseDeviceBasedSatelliteInputLog import com.android.systemui.statusbar.pipeline.satellite.data.RealDeviceBasedSatelliteRepository -import com.android.systemui.statusbar.pipeline.satellite.data.prod.DeviceBasedSatelliteRepositoryImpl.Companion.POLLING_INTERVAL_MS import com.android.systemui.statusbar.pipeline.satellite.data.prod.SatelliteSupport.Companion.whenSupported import com.android.systemui.statusbar.pipeline.satellite.data.prod.SatelliteSupport.NotSupported import com.android.systemui.statusbar.pipeline.satellite.data.prod.SatelliteSupport.Supported @@ -60,11 +60,9 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.collectLatest -import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.flowOn -import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.mapNotNull import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.flow.stateIn @@ -122,15 +120,9 @@ sealed interface SatelliteSupport { } /** - * Basically your everyday run-of-the-mill system service listener, with three notable exceptions. + * Basically your everyday run-of-the-mill system service listener, with two notable exceptions. * - * First, there is an availability bit that we are tracking via [SatelliteManager]. See - * [isSatelliteAllowedForCurrentLocation] for the implementation details. The thing to note about - * this bit is that there is no callback that exists. Therefore we implement a simple polling - * mechanism here. Since the underlying bit is location-dependent, we simply poll every hour (see - * [POLLING_INTERVAL_MS]) and see what the current state is. - * - * Secondly, there are cases when simply requesting information from SatelliteManager can fail. See + * First, there are cases when simply requesting information from SatelliteManager can fail. See * [SatelliteSupport] for details on how we track the state. What's worth noting here is that * SUPPORTED is a stronger guarantee than [satelliteManager] being null. Therefore, the fundamental * data flows here ([connectionState], [signalStrength],...) are wrapped in the convenience method @@ -138,7 +130,7 @@ sealed interface SatelliteSupport { * [SupportedSatelliteManager], we can guarantee that the manager is non-null AND that it has told * us that satellite is supported. Therefore, we don't expect exceptions to be thrown. * - * Lastly, this class is designed to wait a full minute of process uptime before making any requests + * Second, this class is designed to wait a full minute of process uptime before making any requests * to the satellite manager. The hope is that by waiting we don't have to retry due to a modem that * is still booting up or anything like that. We can tune or remove this behavior in the future if * necessary. @@ -158,8 +150,6 @@ constructor( private val satelliteManager: SatelliteManager? - override val isSatelliteAllowedForCurrentLocation: MutableStateFlow<Boolean> - // Some calls into satellite manager will throw exceptions if it is not supported. // This is never expected to change after boot, but may need to be retried in some cases @get:VisibleForTesting @@ -221,8 +211,6 @@ constructor( init { satelliteManager = satelliteManagerOpt.getOrNull() - isSatelliteAllowedForCurrentLocation = MutableStateFlow(false) - if (satelliteManager != null) { // Outer scope launch allows us to delay until MIN_UPTIME scope.launch { @@ -233,10 +221,7 @@ constructor( { "Checked for system support. support=$str1" }, ) - // Second, launch a job to poll for service availability based on location - scope.launch { pollForAvailabilityBasedOnLocation() } - - // Third, register a listener to let us know if there are changes to support + // Second, register a listener to let us know if there are changes to support scope.launch { listenForChangesToSatelliteSupport(satelliteManager) } } } else { @@ -259,28 +244,43 @@ constructor( return sm.checkSatelliteSupported() } - /* - * As there is no listener available for checking satellite allowed, we must poll the service. - * Defaulting to polling at most once every 20m while active. Subsequent OOS events will restart - * the job, so a flaky connection might cause more frequent checks. - */ - private suspend fun pollForAvailabilityBasedOnLocation() { + override val isSatelliteAllowedForCurrentLocation = satelliteSupport .whenSupported( - supported = ::isSatelliteAllowedHasListener, + supported = ::isSatelliteAvailableFlow, orElse = flowOf(false), retrySignal = telephonyProcessCrashedEvent, ) - .collectLatest { hasSubscribers -> - if (hasSubscribers) { - while (true) { - logBuffer.i { "requestIsCommunicationAllowedForCurrentLocation" } - checkIsSatelliteAllowed() - delay(POLLING_INTERVAL_MS) + .stateIn(scope, SharingStarted.Lazily, false) + + private fun isSatelliteAvailableFlow(sm: SupportedSatelliteManager): Flow<Boolean> = + conflatedCallbackFlow { + val callback = SatelliteCommunicationAllowedStateCallback { allowed -> + logBuffer.i({ bool1 = allowed }) { + "onSatelliteCommunicationAllowedStateChanged: $bool1" + } + + trySend(allowed) + } + + var registered = false + try { + sm.registerForCommunicationAllowedStateChanged( + bgDispatcher.asExecutor(), + callback + ) + registered = true + } catch (e: Exception) { + logBuffer.e("Error calling registerForCommunicationAllowedStateChanged", e) + } + + awaitClose { + if (registered) { + sm.unregisterForCommunicationAllowedStateChanged(callback) } } } - } + .flowOn(bgDispatcher) /** * Register a callback with [SatelliteManager] to let us know if there is a change in satellite @@ -410,14 +410,6 @@ constructor( } } - /** - * Signal that we should start polling [checkIsSatelliteAllowed]. We only need to poll if there - * are active listeners to [isSatelliteAllowedForCurrentLocation] - */ - @SuppressWarnings("unused") - private fun isSatelliteAllowedHasListener(sm: SupportedSatelliteManager): Flow<Boolean> = - isSatelliteAllowedForCurrentLocation.subscriptionCount.map { it > 0 }.distinctUntilChanged() - override val connectionState = satelliteSupport .whenSupported( @@ -485,28 +477,6 @@ constructor( } .flowOn(bgDispatcher) - /** Fire off a request to check for satellite availability. Always runs on the bg context */ - private suspend fun checkIsSatelliteAllowed() = - withContext(bgDispatcher) { - satelliteManager?.requestIsCommunicationAllowedForCurrentLocation( - bgDispatcher.asExecutor(), - object : OutcomeReceiver<Boolean, SatelliteManager.SatelliteException> { - override fun onError(e: SatelliteManager.SatelliteException) { - logBuffer.e( - "Found exception when checking availability", - e, - ) - isSatelliteAllowedForCurrentLocation.value = false - } - - override fun onResult(allowed: Boolean) { - logBuffer.i { "isSatelliteAllowedForCurrentLocation: $allowed" } - isSatelliteAllowedForCurrentLocation.value = allowed - } - } - ) - } - private suspend fun SatelliteManager.checkSatelliteSupported(): SatelliteSupport = suspendCancellableCoroutine { continuation -> val cb = @@ -546,9 +516,6 @@ constructor( } companion object { - // TTL for satellite polling is twenty minutes - const val POLLING_INTERVAL_MS: Long = 1000 * 60 * 20 - // Let the system boot up and stabilize before we check for system support const val MIN_UPTIME: Long = 1000 * 60 diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/data/prod/DeviceBasedSatelliteRepositoryImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/data/prod/DeviceBasedSatelliteRepositoryImplTest.kt index 73ac6e3573d6..af4f647923a7 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/data/prod/DeviceBasedSatelliteRepositoryImplTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/satellite/data/prod/DeviceBasedSatelliteRepositoryImplTest.kt @@ -22,6 +22,7 @@ import android.telephony.TelephonyCallback import android.telephony.TelephonyManager import android.telephony.satellite.NtnSignalStrength import android.telephony.satellite.NtnSignalStrengthCallback +import android.telephony.satellite.SatelliteCommunicationAllowedStateCallback import android.telephony.satellite.SatelliteManager import android.telephony.satellite.SatelliteManager.SATELLITE_MODEM_STATE_CONNECTED import android.telephony.satellite.SatelliteManager.SATELLITE_MODEM_STATE_DATAGRAM_RETRYING @@ -44,7 +45,6 @@ import com.android.systemui.coroutines.collectLastValue import com.android.systemui.log.core.FakeLogBuffer import com.android.systemui.statusbar.pipeline.mobile.data.repository.prod.MobileTelephonyHelpers import com.android.systemui.statusbar.pipeline.satellite.data.prod.DeviceBasedSatelliteRepositoryImpl.Companion.MIN_UPTIME -import com.android.systemui.statusbar.pipeline.satellite.data.prod.DeviceBasedSatelliteRepositoryImpl.Companion.POLLING_INTERVAL_MS import com.android.systemui.statusbar.pipeline.satellite.shared.model.SatelliteConnectionState import com.android.systemui.util.mockito.any import com.android.systemui.util.mockito.whenever @@ -54,11 +54,8 @@ import com.google.common.truth.Truth.assertThat import java.util.Optional import kotlin.test.Test import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope -import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.Before @@ -71,6 +68,7 @@ import org.mockito.Mockito.never import org.mockito.Mockito.times import org.mockito.Mockito.verify import org.mockito.MockitoAnnotations +import org.mockito.kotlin.doThrow @Suppress("EXPERIMENTAL_IS_NOT_ENABLED") @OptIn(ExperimentalCoroutinesApi::class) @@ -186,149 +184,83 @@ class DeviceBasedSatelliteRepositoryImplTest : SysuiTestCase() { } @Test - fun isSatelliteAllowed_readsSatelliteManagerState_enabled() = + fun isSatelliteAllowed_listensToSatelliteManagerCallback() = testScope.runTest { setupDefaultRepo() - // GIVEN satellite is allowed in this location - val allowed = true - - doAnswer { - val receiver = it.arguments[1] as OutcomeReceiver<Boolean, SatelliteException> - receiver.onResult(allowed) - null - } - .`when`(satelliteManager) - .requestIsCommunicationAllowedForCurrentLocation( - any(), - any<OutcomeReceiver<Boolean, SatelliteException>>() - ) val latest by collectLastValue(underTest.isSatelliteAllowedForCurrentLocation) + runCurrent() - assertThat(latest).isTrue() - } - - @Test - fun isSatelliteAllowed_readsSatelliteManagerState_disabled() = - testScope.runTest { - setupDefaultRepo() - // GIVEN satellite is not allowed in this location - val allowed = false - - doAnswer { - val receiver = it.arguments[1] as OutcomeReceiver<Boolean, SatelliteException> - receiver.onResult(allowed) - null + val callback = + withArgCaptor<SatelliteCommunicationAllowedStateCallback> { + verify(satelliteManager) + .registerForCommunicationAllowedStateChanged(any(), capture()) } - .`when`(satelliteManager) - .requestIsCommunicationAllowedForCurrentLocation( - any(), - any<OutcomeReceiver<Boolean, SatelliteException>>() - ) - val latest by collectLastValue(underTest.isSatelliteAllowedForCurrentLocation) + // WHEN satellite manager says it's not available + callback.onSatelliteCommunicationAllowedStateChanged(false) + // THEN it's not! assertThat(latest).isFalse() + + // WHEN satellite manager says it's changed to available + callback.onSatelliteCommunicationAllowedStateChanged(true) + + // THEN it is! + assertThat(latest).isTrue() } @Test - fun isSatelliteAllowed_pollsOnTimeout() = + fun isSatelliteAllowed_falseWhenErrorOccurs() = testScope.runTest { setupDefaultRepo() - // GIVEN satellite is not allowed in this location - var allowed = false - doAnswer { - val receiver = it.arguments[1] as OutcomeReceiver<Boolean, SatelliteException> - receiver.onResult(allowed) - null - } + // GIVEN SatelliteManager gon' throw exceptions when we ask to register the callback + doThrow(RuntimeException("Test exception")) .`when`(satelliteManager) - .requestIsCommunicationAllowedForCurrentLocation( - any(), - any<OutcomeReceiver<Boolean, SatelliteException>>() - ) + .registerForCommunicationAllowedStateChanged(any(), any()) + // WHEN the latest value is requested (and thus causes an exception to be thrown) val latest by collectLastValue(underTest.isSatelliteAllowedForCurrentLocation) + // THEN the value is just false, and we didn't crash! assertThat(latest).isFalse() - - // WHEN satellite becomes enabled - allowed = true - - // WHEN the timeout has not yet been reached - advanceTimeBy(POLLING_INTERVAL_MS / 2) - - // THEN the value is still false - assertThat(latest).isFalse() - - // WHEN time advances beyond the polling interval - advanceTimeBy(POLLING_INTERVAL_MS / 2 + 1) - - // THEN then new value is emitted - assertThat(latest).isTrue() } @Test - fun isSatelliteAllowed_pollingRestartsWhenCollectionRestarts() = + fun isSatelliteAllowed_reRegistersOnTelephonyProcessCrash() = testScope.runTest { setupDefaultRepo() - // Use the old school launch/cancel so we can simulate subscribers arriving and leaving - - var latest: Boolean? = false - var job = - underTest.isSatelliteAllowedForCurrentLocation.onEach { latest = it }.launchIn(this) - - // GIVEN satellite is not allowed in this location - var allowed = false + val latest by collectLastValue(underTest.isSatelliteAllowedForCurrentLocation) + runCurrent() - doAnswer { - val receiver = it.arguments[1] as OutcomeReceiver<Boolean, SatelliteException> - receiver.onResult(allowed) - null + val callback = + withArgCaptor<SatelliteCommunicationAllowedStateCallback> { + verify(satelliteManager) + .registerForCommunicationAllowedStateChanged(any(), capture()) } - .`when`(satelliteManager) - .requestIsCommunicationAllowedForCurrentLocation( - any(), - any<OutcomeReceiver<Boolean, SatelliteException>>() - ) - assertThat(latest).isFalse() - - // WHEN satellite becomes enabled - allowed = true - - // WHEN the job is restarted - advanceTimeBy(POLLING_INTERVAL_MS / 2) + val telephonyCallback = + MobileTelephonyHelpers.getTelephonyCallbackForType< + TelephonyCallback.RadioPowerStateListener + >( + telephonyManager + ) - job.cancel() - job = - underTest.isSatelliteAllowedForCurrentLocation.onEach { latest = it }.launchIn(this) + // GIVEN satellite is currently provisioned + callback.onSatelliteCommunicationAllowedStateChanged(true) - // THEN the value is re-fetched assertThat(latest).isTrue() - job.cancel() - } - - @Test - fun isSatelliteAllowed_falseWhenErrorOccurs() = - testScope.runTest { - setupDefaultRepo() - doAnswer { - val receiver = it.arguments[1] as OutcomeReceiver<Boolean, SatelliteException> - receiver.onError(SatelliteException(1 /* unused */)) - null - } - .`when`(satelliteManager) - .requestIsCommunicationAllowedForCurrentLocation( - any(), - any<OutcomeReceiver<Boolean, SatelliteException>>() - ) - - val latest by collectLastValue(underTest.isSatelliteAllowedForCurrentLocation) + // WHEN a crash event happens (detected by radio state change) + telephonyCallback.onRadioPowerStateChanged(TelephonyManager.RADIO_POWER_ON) + runCurrent() + telephonyCallback.onRadioPowerStateChanged(TelephonyManager.RADIO_POWER_OFF) + runCurrent() - assertThat(latest).isFalse() + // THEN listener is re-registered + verify(satelliteManager, times(2)) + .registerForCommunicationAllowedStateChanged(any(), any()) } @Test @@ -363,24 +295,21 @@ class DeviceBasedSatelliteRepositoryImplTest : SysuiTestCase() { testScope.runTest { // GIVEN satellite is supported on device doAnswer { - val callback: OutcomeReceiver<Boolean, SatelliteException> = - it.getArgument(1) as OutcomeReceiver<Boolean, SatelliteException> - callback.onResult(true) - } + val callback: OutcomeReceiver<Boolean, SatelliteException> = + it.getArgument(1) as OutcomeReceiver<Boolean, SatelliteException> + callback.onResult(true) + } .whenever(satelliteManager) .requestIsSupported(any(), any()) // GIVEN satellite returns an error when asked if provisioned doAnswer { - val receiver = it.arguments[1] as OutcomeReceiver<Boolean, SatelliteException> - receiver.onError(SatelliteException(SATELLITE_RESULT_ERROR)) - null - } + val receiver = it.arguments[1] as OutcomeReceiver<Boolean, SatelliteException> + receiver.onError(SatelliteException(SATELLITE_RESULT_ERROR)) + null + } .whenever(satelliteManager) - .requestIsProvisioned( - any(), - any<OutcomeReceiver<Boolean, SatelliteException>>() - ) + .requestIsProvisioned(any(), any<OutcomeReceiver<Boolean, SatelliteException>>()) // GIVEN we've been up long enough to start querying systemClock.setUptimeMillis(Process.getStartUptimeMillis() + MIN_UPTIME) @@ -409,10 +338,10 @@ class DeviceBasedSatelliteRepositoryImplTest : SysuiTestCase() { testScope.runTest { // GIVEN satellite is supported on device doAnswer { - val callback: OutcomeReceiver<Boolean, SatelliteException> = - it.getArgument(1) as OutcomeReceiver<Boolean, SatelliteException> - callback.onResult(true) - } + val callback: OutcomeReceiver<Boolean, SatelliteException> = + it.getArgument(1) as OutcomeReceiver<Boolean, SatelliteException> + callback.onResult(true) + } .whenever(satelliteManager) .requestIsSupported(any(), any()) @@ -779,10 +708,10 @@ class DeviceBasedSatelliteRepositoryImplTest : SysuiTestCase() { .requestIsSupported(any(), any()) doAnswer { - val callback: OutcomeReceiver<Boolean, SatelliteException> = - it.getArgument(1) as OutcomeReceiver<Boolean, SatelliteException> - callback.onResult(initialSatelliteIsProvisioned) - } + val callback: OutcomeReceiver<Boolean, SatelliteException> = + it.getArgument(1) as OutcomeReceiver<Boolean, SatelliteException> + callback.onResult(initialSatelliteIsProvisioned) + } .whenever(satelliteManager) .requestIsProvisioned(any(), any()) |