refactor: Reconnecting channels is a EuiccChannelManager responsibility

Reconnecting did not work properly for OMAPI, because in that case we
have to reconnect SEService as well.

app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelFactory.kt

@@ -10,7 +10,7 @@ open class DefaultEuiccChannelFactory(protected val context: Context) : EuiccCha
     private var seService: SEService? = null
 
     private suspend fun ensureSEService() {
-        if (seService == null) {
+        if (seService == null || !seService!!.isConnected) {
             seService = connectSEService(context)
         }
     }

app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelManager.kt

@@ -6,10 +6,12 @@ import android.util.Log
 import im.angry.openeuicc.di.AppContainer
 import im.angry.openeuicc.util.*
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.delay
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 import kotlinx.coroutines.withContext
+import kotlinx.coroutines.withTimeout
 
 open class DefaultEuiccChannelManager(
     protected val appContainer: AppContainer,
@@ -87,14 +89,18 @@ open class DefaultEuiccChannelManager(
             }
         }
 
+    override suspend fun findAllEuiccChannelsByPhysicalSlot(physicalSlotId: Int): List<EuiccChannel>? {
+        for (card in uiccCards) {
+            if (card.physicalSlotIndex != physicalSlotId) continue
+            return card.ports.mapNotNull { tryOpenEuiccChannel(it) }
+                .ifEmpty { null }
+        }
+        return null
+    }
+
     override fun findAllEuiccChannelsByPhysicalSlotBlocking(physicalSlotId: Int): List<EuiccChannel>? =
         runBlocking {
-            for (card in uiccCards) {
-                if (card.physicalSlotIndex != physicalSlotId) continue
-                return@runBlocking card.ports.mapNotNull { tryOpenEuiccChannel(it) }
-                    .ifEmpty { null }
-            }
-            return@runBlocking null
+            findAllEuiccChannelsByPhysicalSlot(physicalSlotId)
         }
 
     override fun findEuiccChannelByPortBlocking(physicalSlotId: Int, portId: Int): EuiccChannel? =
@@ -106,6 +112,25 @@ open class DefaultEuiccChannelManager(
             }
         }
 
+    override suspend fun tryReconnectSlot(physicalSlotId: Int, timeoutMillis: Long) {
+        invalidateByPhysicalSlot(physicalSlotId)
+
+        withTimeout(timeoutMillis) {
+            while (true) {
+                try {
+                    findAllEuiccChannelsByPhysicalSlot(physicalSlotId)!!.forEach {
+                        check(it.valid) { "Invalid channel" }
+                    }
+                    break
+                } catch (e: Exception) {
+                    Log.d(TAG, "Slot reconnect failure, retrying in 1000 ms")
+                    invalidateByPhysicalSlot(physicalSlotId)
+                }
+                delay(1000)
+            }
+        }
+    }
+
     override suspend fun enumerateEuiccChannels() {
         withContext(Dispatchers.IO) {
             for (uiccInfo in uiccCards) {
@@ -132,4 +157,12 @@ open class DefaultEuiccChannelManager(
         channels.clear()
         euiccChannelFactory.cleanup()
     }
+
+    private fun invalidateByPhysicalSlot(physicalSlotId: Int) {
+        val toRemove = channels.filter { it.valid && it.slotId == physicalSlotId }
+        for (channel in toRemove) {
+            channel.close()
+            channels.remove(channel)
+        }
+    }
 }

app-common/src/main/java/im/angry/openeuicc/core/EuiccChannelManager.kt

@@ -8,6 +8,15 @@ interface EuiccChannelManager {
      */
     suspend fun enumerateEuiccChannels()
 
+    /**
+     * Reconnect ALL EuiccChannels belonging to a physical slot
+     * Throws TimeoutCancellationException when timed out
+     * If this operation times out, none of the channels belonging to the slot will be
+     * guaranteed to be consistent. The caller should either call invalidate()
+     * and try again later, or the application should simply exit entirely.
+     */
+    suspend fun tryReconnectSlot(physicalSlotId: Int, timeoutMillis: Long = 1000)
+
     /**
      * Returns the EuiccChannel corresponding to a **logical** slot
      */
@@ -24,6 +33,7 @@ interface EuiccChannelManager {
      * Returns all EuiccChannels corresponding to a **physical** slot
      * Multiple channels are possible in the case of MEP
      */
+    suspend fun findAllEuiccChannelsByPhysicalSlot(physicalSlotId: Int): List<EuiccChannel>?
     fun findAllEuiccChannelsByPhysicalSlotBlocking(physicalSlotId: Int): List<EuiccChannel>?
 
     /**

app-common/src/main/java/im/angry/openeuicc/ui/EuiccManagementFragment.kt

@@ -31,7 +31,6 @@ import kotlinx.coroutines.TimeoutCancellationException
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
-import java.lang.Exception
 
 open class EuiccManagementFragment : Fragment(), EuiccProfilesChangedListener,
     EuiccChannelFragmentMarker {
@@ -132,48 +131,51 @@ open class EuiccManagementFragment : Fragment(), EuiccProfilesChangedListener,
         fab.isEnabled = false
 
         lifecycleScope.launch {
-            try {
-                if (enable) {
-                    doEnableProfile(iccid)
+            beginTrackedOperation {
+                val res = if (enable) {
+                    channel.lpa.enableProfile(iccid)
                 } else {
-                    doDisableProfile(iccid)
+                    channel.lpa.disableProfile(iccid)
                 }
-                refresh()
-                fab.isEnabled = true
-            } catch (e: TimeoutCancellationException) {
-                // Timed out waiting for SIM to come back online, we can no longer assume that the LPA is still valid
-                AlertDialog.Builder(requireContext()).apply {
-                    setMessage(R.string.enable_disable_timeout)
-                    setPositiveButton(android.R.string.ok) { dialog, _ ->
-                        dialog.dismiss()
-                        requireActivity().finish()
-                    }
-                    setOnDismissListener { _ ->
-                        requireActivity().finish()
-                    }
-                    show()
+
+                if (!res) {
+                    Log.d(TAG, "Failed to enable / disable profile $iccid")
+                    Toast.makeText(context, R.string.toast_profile_enable_failed, Toast.LENGTH_LONG)
+                        .show()
+                    return@beginTrackedOperation false
+                }
+
+                try {
+                    euiccChannelManager.tryReconnectSlot(slotId, timeoutMillis = 30 * 1000)
+                } catch (e: TimeoutCancellationException) {
+                    withContext(Dispatchers.Main) {
+                        // Timed out waiting for SIM to come back online, we can no longer assume that the LPA is still valid
+                        AlertDialog.Builder(requireContext()).apply {
+                            setMessage(R.string.enable_disable_timeout)
+                            setPositiveButton(android.R.string.ok) { dialog, _ ->
+                                dialog.dismiss()
+                                requireActivity().finish()
+                            }
+                            setOnDismissListener { _ ->
+                                requireActivity().finish()
+                            }
+                            show()
+                        }
+                    }
+                    return@beginTrackedOperation false
+                }
+
+                if (enable) {
+                    preferenceRepository.notificationEnableFlow.first()
+                } else {
+                    preferenceRepository.notificationDisableFlow.first()
                 }
-            } catch (e: Exception) {
-                Log.d(TAG, "Failed to enable / disable profile $iccid")
-                Log.d(TAG, Log.getStackTraceString(e))
-                fab.isEnabled = true
-                Toast.makeText(context, R.string.toast_profile_enable_failed, Toast.LENGTH_LONG).show()
             }
+            refresh()
+            fab.isEnabled = true
         }
     }
 
-    private suspend fun doEnableProfile(iccid: String) =
-        beginTrackedOperation {
-            channel.lpa.enableProfile(iccid, reconnectTimeout = 15 * 1000) &&
-                preferenceRepository.notificationEnableFlow.first()
-        }
-
-    private suspend fun doDisableProfile(iccid: String) =
-        beginTrackedOperation {
-            channel.lpa.disableProfile(iccid, reconnectTimeout = 15 * 1000) &&
-                preferenceRepository.notificationDisableFlow.first()
-        }
-
     protected open fun populatePopupWithProfileActions(popup: PopupMenu, profile: LocalProfileInfo) {
         popup.inflate(R.menu.profile_options)
         if (profile.isEnabled) {

app-unpriv/src/main/java/im/angry/openeuicc/UnprivilegedOpenEuiccApplication.kt

@@ -15,7 +15,8 @@ class UnprivilegedOpenEuiccApplication : OpenEuiccApplication() {
     override fun onCreate() {
         super.onCreate()
 
-        Thread.setDefaultUncaughtExceptionHandler { _, _ ->
+        Thread.setDefaultUncaughtExceptionHandler { _, e ->
+            e.printStackTrace()
             Intent(this, LogsActivity::class.java).apply {
                 addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK)
                 addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)

libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/LocalProfileAssistant.kt

@@ -19,8 +19,8 @@ interface LocalProfileAssistant {
 
     // All blocking functions in this class assume that they are executed on non-Main threads
     // The IO context in Kotlin's coroutine library is recommended.
-    fun enableProfile(iccid: String, reconnectTimeout: Long = 0): Boolean
-    fun disableProfile(iccid: String, reconnectTimeout: Long = 0): Boolean
+    fun enableProfile(iccid: String): Boolean
+    fun disableProfile(iccid: String): Boolean
     fun deleteProfile(iccid: String): Boolean
 
     fun downloadProfile(smdp: String, matchingId: String?, imei: String?,

libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt

@@ -1,9 +1,6 @@
 package net.typeblog.lpac_jni.impl
 
 import android.util.Log
-import kotlinx.coroutines.delay
-import kotlinx.coroutines.runBlocking
-import kotlinx.coroutines.withTimeout
 import net.typeblog.lpac_jni.LpacJni
 import net.typeblog.lpac_jni.ApduInterface
 import net.typeblog.lpac_jni.EuiccInfo2
@@ -14,8 +11,8 @@ import net.typeblog.lpac_jni.LocalProfileNotification
 import net.typeblog.lpac_jni.ProfileDownloadCallback
 
 class LocalProfileAssistantImpl(
-    private val apduInterface: ApduInterface,
-    private val httpInterface: HttpInterface
+    apduInterface: ApduInterface,
+    httpInterface: HttpInterface
 ): LocalProfileAssistant {
     companion object {
         private const val TAG = "LocalProfileAssistantImpl"
@@ -31,48 +28,6 @@ class LocalProfileAssistantImpl(
         httpInterface.usePublicKeyIds(pkids)
     }
 
-    private fun tryReconnect(timeoutMillis: Long) = runBlocking {
-        withTimeout(timeoutMillis) {
-            try {
-                LpacJni.euiccFini(contextHandle)
-                LpacJni.destroyContext(contextHandle)
-                contextHandle = -1
-            } catch (e: Exception) {
-                // Ignored
-            }
-
-            while (true) {
-                try {
-                    apduInterface.disconnect()
-                } catch (e: Exception) {
-                    // Ignored
-                }
-
-                try {
-                    apduInterface.connect()
-                    contextHandle = LpacJni.createContext(apduInterface, httpInterface)
-                    check(LpacJni.euiccInit(contextHandle) >= 0) { "Reconnect attempt failed" }
-                    // Validate that we can actually use the APDU channel by trying to read eID and profiles
-                    check(valid) { "Reconnected channel is invalid" }
-                    break
-                } catch (e: Exception) {
-                    e.printStackTrace()
-                    if (contextHandle != -1L) {
-                        try {
-                            LpacJni.euiccFini(contextHandle)
-                            LpacJni.destroyContext(contextHandle)
-                            contextHandle = -1
-                        } catch (e: Exception) {
-                            // Ignored
-                        }
-                    }
-                    // continue retrying
-                    delay(1000)
-                }
-            }
-        }
-    }
-
     override val profiles: List<LocalProfileInfo>
         get() = LpacJni.es10cGetProfilesInfo(contextHandle)!!.asList()
 
@@ -87,21 +42,11 @@ class LocalProfileAssistantImpl(
     override val euiccInfo2: EuiccInfo2?
         get() = LpacJni.es10cexGetEuiccInfo2(contextHandle)
 
-    override fun enableProfile(iccid: String, reconnectTimeout: Long): Boolean {
-        val res = LpacJni.es10cEnableProfile(contextHandle, iccid) == 0
-        if (reconnectTimeout > 0) {
-            tryReconnect(reconnectTimeout)
-        }
-        return res
-    }
+    override fun enableProfile(iccid: String): Boolean =
+        LpacJni.es10cEnableProfile(contextHandle, iccid) == 0
 
-    override fun disableProfile(iccid: String, reconnectTimeout: Long): Boolean {
-        val res = LpacJni.es10cDisableProfile(contextHandle, iccid) == 0
-        if (reconnectTimeout > 0) {
-            tryReconnect(reconnectTimeout)
-        }
-        return res
-    }
+    override fun disableProfile(iccid: String): Boolean =
+        LpacJni.es10cDisableProfile(contextHandle, iccid) == 0
 
     override fun deleteProfile(iccid: String): Boolean {
         return LpacJni.es10cDeleteProfile(contextHandle, iccid) == 0