Revert^2 "[Sat] Add satellite supported changed callback"

This reverts commit aa0f68b0.

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

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<SatelliteSupport> = 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<Boolean> =
+        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<Boolean> =
+        isSatelliteAllowedForCurrentLocation.subscriptionCount.map { it > 0 }.distinctUntilChanged()
+
     override val connectionState =
         satelliteSupport
             .whenSupported(

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<SatelliteSupportedStateCallback> {
+                    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<SatelliteSupportedStateCallback> {
+                    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 {