From cdb647220b9444f0a974b5fdc9ac5bec133af6da Mon Sep 17 00:00:00 2001 From: Evan Laird Date: Wed, 29 May 2024 18:53:30 +0000 Subject: Revert^2 "[Sat] Add satellite supported changed callback" This reverts commit aa0f68b0ab32714cffe884d4d3f4b0de07b6bd52. Reason for revert: Added a a try/catch to handle the exception Original commit message: Per the linked bug, there is a new API on `SatelliteManager` that implements a callback for satellite support. This is in addition to the existing query API that happens when the repository starts up. This CL adds the listener after the initial query, and makes sure that if satellite support shows up later it is caught. The state change listener runs regardless of the initial supported state. Test: DeviceBasedSatelliteRepositoryImplTest Bug: 342278770 Flag: NONE bugfix Change-Id: I2ddf754b60ca71427754c3940f593e851c9c28da Merged-In: I2ddf754b60ca71427754c3940f593e851c9c28da --- .../prod/DeviceBasedSatelliteRepositoryImpl.kt | 184 +++++++++++++++------ .../prod/DeviceBasedSatelliteRepositoryImplTest.kt | 106 +++++++++++- 2 files changed, 235 insertions(+), 55 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 0ccfd11c1d96..2138b9928bed 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 @@ -23,6 +23,7 @@ import android.telephony.satellite.NtnSignalStrengthCallback import android.telephony.satellite.SatelliteManager import android.telephony.satellite.SatelliteManager.SATELLITE_RESULT_SUCCESS import android.telephony.satellite.SatelliteModemStateCallback +import android.telephony.satellite.SatelliteSupportedStateCallback import androidx.annotation.VisibleForTesting import com.android.systemui.common.coroutine.ConflatedCallbackFlow.conflatedCallbackFlow import com.android.systemui.dagger.SysUISingleton @@ -35,6 +36,7 @@ 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.DeviceBasedSatelliteRepository +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 @@ -162,60 +164,6 @@ constructor( @get:VisibleForTesting val satelliteSupport: MutableStateFlow = MutableStateFlow(Unknown) - init { - satelliteManager = satelliteManagerOpt.getOrNull() - - isSatelliteAllowedForCurrentLocation = MutableStateFlow(false) - - if (satelliteManager != null) { - // First, check that satellite is supported on this device - scope.launch { - val waitTime = ensureMinUptime(systemClock, MIN_UPTIME) - if (waitTime > 0) { - logBuffer.i({ long1 = waitTime }) { - "Waiting $long1 ms before checking for satellite support" - } - delay(waitTime) - } - - satelliteSupport.value = satelliteManager.checkSatelliteSupported() - - logBuffer.i( - { str1 = satelliteSupport.value.toString() }, - { "Checked for system support. support=$str1" }, - ) - - // We only need to check location availability if this mode is supported - if (satelliteSupport.value is Supported) { - isSatelliteAllowedForCurrentLocation.subscriptionCount - .map { it > 0 } - .distinctUntilChanged() - .collectLatest { hasSubscribers -> - if (hasSubscribers) { - /* - * As there is no listener available for checking satellite allowed, - * we must poll. Defaulting to polling at most once every hour while - * active. Subsequent OOS events will restart the job, so a flaky - * connection might cause more frequent checks. - */ - while (true) { - logBuffer.i { - "requestIsCommunicationAllowedForCurrentLocation" - } - checkIsSatelliteAllowed() - delay(POLLING_INTERVAL_MS) - } - } - } - } - } - } else { - logBuffer.i { "Satellite manager is null" } - - satelliteSupport.value = NotSupported - } - } - /** * Note that we are given an "unbound" [TelephonyManager] (meaning it was not created with a * specific `subscriptionId`). Therefore this is the radio power state of the @@ -269,6 +217,134 @@ constructor( } .onStart { emit(Unit) } + init { + satelliteManager = satelliteManagerOpt.getOrNull() + + isSatelliteAllowedForCurrentLocation = MutableStateFlow(false) + + if (satelliteManager != null) { + // Outer scope launch allows us to delay until MIN_UPTIME + scope.launch { + // First, check that satellite is supported on this device + satelliteSupport.value = checkSatelliteSupportAfterMinUptime(satelliteManager) + logBuffer.i( + { str1 = satelliteSupport.value.toString() }, + { "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 + scope.launch { listenForChangesToSatelliteSupport(satelliteManager) } + } + } else { + logBuffer.i { "Satellite manager is null" } + satelliteSupport.value = NotSupported + } + } + + private suspend fun checkSatelliteSupportAfterMinUptime( + sm: SatelliteManager + ): SatelliteSupport { + val waitTime = ensureMinUptime(systemClock, MIN_UPTIME) + if (waitTime > 0) { + logBuffer.i({ long1 = waitTime }) { + "Waiting $long1 ms before checking for satellite support" + } + delay(waitTime) + } + + 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() { + satelliteSupport + .whenSupported( + supported = ::isSatelliteAllowedHasListener, + orElse = flowOf(false), + retrySignal = telephonyProcessCrashedEvent, + ) + .collectLatest { hasSubscribers -> + if (hasSubscribers) { + while (true) { + logBuffer.i { "requestIsCommunicationAllowedForCurrentLocation" } + checkIsSatelliteAllowed() + delay(POLLING_INTERVAL_MS) + } + } + } + } + + /** + * Register a callback with [SatelliteManager] to let us know if there is a change in satellite + * support. This job restarts if there is a crash event detected. + * + * Note that the structure of this method looks similar to [whenSupported], but since we want + * this callback registered even when it is [NotSupported], we just mimic the structure here. + */ + private suspend fun listenForChangesToSatelliteSupport(sm: SatelliteManager) { + telephonyProcessCrashedEvent.collectLatest { + satelliteIsSupportedCallback.collect { supported -> + if (supported) { + satelliteSupport.value = Supported(sm) + } else { + satelliteSupport.value = NotSupported + } + } + } + } + + /** + * Callback version of [checkSatelliteSupported]. This flow should be retried on the same + * [telephonyProcessCrashedEvent] signal, but does not require a [SupportedSatelliteManager], + * since it specifically watches for satellite support. + */ + private val satelliteIsSupportedCallback: Flow = + if (satelliteManager == null) { + flowOf(false) + } else { + conflatedCallbackFlow { + val callback = SatelliteSupportedStateCallback { supported -> + logBuffer.i { + "onSatelliteSupportedStateChanged: " + + "${if (supported) "supported" else "not supported"}" + } + trySend(supported) + } + + var registered = false + try { + satelliteManager.registerForSupportedStateChanged( + bgDispatcher.asExecutor(), + callback + ) + registered = true + } catch (e: Exception) { + logBuffer.e("error registering for supported state change", e) + } + + awaitClose { + if (registered) { + satelliteManager.unregisterForSupportedStateChanged(callback) + } + } + } + } + + /** + * 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 = + isSatelliteAllowedForCurrentLocation.subscriptionCount.map { it > 0 }.distinctUntilChanged() + override val connectionState = satelliteSupport .whenSupported( 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 34fccb0ab51d..a2b31e23bfb3 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 @@ -34,6 +34,7 @@ import android.telephony.satellite.SatelliteManager.SATELLITE_MODEM_STATE_UNAVAI import android.telephony.satellite.SatelliteManager.SATELLITE_MODEM_STATE_UNKNOWN import android.telephony.satellite.SatelliteManager.SatelliteException import android.telephony.satellite.SatelliteModemStateCallback +import android.telephony.satellite.SatelliteSupportedStateCallback import androidx.test.filters.SmallTest import com.android.systemui.SysuiTestCase import com.android.systemui.coroutines.collectLastValue @@ -327,7 +328,6 @@ class DeviceBasedSatelliteRepositoryImplTest : SysuiTestCase() { @Test fun satelliteNotSupported_listenersAreNotRegistered() = testScope.runTest { - setupDefaultRepo() // GIVEN satellite is not supported setUpRepo( uptime = MIN_UPTIME, @@ -344,6 +344,110 @@ class DeviceBasedSatelliteRepositoryImplTest : SysuiTestCase() { verify(satelliteManager, never()).registerForNtnSignalStrengthChanged(any(), any()) } + @Test + fun satelliteSupported_registersCallbackForStateChanges() = + testScope.runTest { + // GIVEN a supported satellite manager. + setupDefaultRepo() + runCurrent() + + // THEN the repo registers for state changes of satellite support + verify(satelliteManager, times(1)).registerForSupportedStateChanged(any(), any()) + } + + @Test + fun satelliteNotSupported_registersCallbackForStateChanges() = + testScope.runTest { + // GIVEN satellite is not supported + setUpRepo( + uptime = MIN_UPTIME, + satMan = satelliteManager, + satelliteSupported = false, + ) + + runCurrent() + // THEN the repo registers for state changes of satellite support + verify(satelliteManager, times(1)).registerForSupportedStateChanged(any(), any()) + } + + @Test + fun satelliteSupportedStateChangedCallbackThrows_doesNotCrash() = + testScope.runTest { + // GIVEN, satellite manager throws when registering for supported state changes + whenever(satelliteManager.registerForSupportedStateChanged(any(), any())) + .thenThrow(IllegalStateException()) + + // GIVEN a supported satellite manager. + setupDefaultRepo() + runCurrent() + + // THEN a listener for satellite supported changed can attempt to register, + // with no crash + verify(satelliteManager).registerForSupportedStateChanged(any(), any()) + } + + @Test + fun satelliteSupported_supportIsLost_unregistersListeners() = + testScope.runTest { + // GIVEN a supported satellite manager. + setupDefaultRepo() + runCurrent() + + val callback = + withArgCaptor { + verify(satelliteManager).registerForSupportedStateChanged(any(), capture()) + } + + // WHEN data is requested from the repo + val connectionState by collectLastValue(underTest.connectionState) + val signalStrength by collectLastValue(underTest.signalStrength) + + // THEN the listeners are registered + verify(satelliteManager, times(1)).registerForModemStateChanged(any(), any()) + verify(satelliteManager, times(1)).registerForNtnSignalStrengthChanged(any(), any()) + + // WHEN satellite support turns off + callback.onSatelliteSupportedStateChanged(false) + runCurrent() + + // THEN listeners are unregistered + verify(satelliteManager, times(1)).unregisterForModemStateChanged(any()) + verify(satelliteManager, times(1)).unregisterForNtnSignalStrengthChanged(any()) + } + + @Test + fun satelliteNotSupported_supportShowsUp_registersListeners() = + testScope.runTest { + // GIVEN satellite is not supported + setUpRepo( + uptime = MIN_UPTIME, + satMan = satelliteManager, + satelliteSupported = false, + ) + runCurrent() + + val callback = + withArgCaptor { + verify(satelliteManager).registerForSupportedStateChanged(any(), capture()) + } + + // WHEN data is requested from the repo + val connectionState by collectLastValue(underTest.connectionState) + val signalStrength by collectLastValue(underTest.signalStrength) + + // THEN the listeners are not yet registered + verify(satelliteManager, times(0)).registerForModemStateChanged(any(), any()) + verify(satelliteManager, times(0)).registerForNtnSignalStrengthChanged(any(), any()) + + // WHEN satellite support turns on + callback.onSatelliteSupportedStateChanged(true) + runCurrent() + + // THEN listeners are registered + verify(satelliteManager, times(1)).registerForModemStateChanged(any(), any()) + verify(satelliteManager, times(1)).registerForNtnSignalStrengthChanged(any(), any()) + } + @Test fun repoDoesNotCheckForSupportUntilMinUptime() = testScope.runTest { -- cgit v1.2.3-59-g8ed1b