From 31c06470c6e2b1592322d4b30e9988a8c73cd9d8 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sun, 29 Sep 2024 16:49:21 -0400 Subject: [PATCH 1/4] Move profile deletion to new flow --- .../service/EuiccChannelManagerService.kt | 24 +++++++++++++- .../openeuicc/ui/ProfileDeleteFragment.kt | 31 +++++++++---------- .../src/main/res/drawable/ic_task_delete.xml | 5 +++ app-common/src/main/res/values/strings.xml | 1 + 4 files changed, 44 insertions(+), 17 deletions(-) create mode 100644 app-common/src/main/res/drawable/ic_task_delete.xml diff --git a/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt b/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt index c004c5f..6061a94 100644 --- a/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt +++ b/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt @@ -133,11 +133,14 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker { * Launch a potentially blocking foreground task in this service's lifecycle context. * This function does not block, but returns a Flow that emits ForegroundTaskState * updates associated with this task. The last update the returned flow will emit is - * always ForegroundTaskState.Done. + * always ForegroundTaskState.Done. The returned flow MUST be started in order for the + * foreground task to run. * * The task closure is expected to update foregroundTaskState whenever appropriate. * If a foreground task is already running, this function returns null. * + * To wait for foreground tasks to be available, use waitForForegroundTask(). + * * The function will set the state back to Idle once it sees ForegroundTaskState.Done. */ private fun launchForegroundTask( @@ -262,4 +265,23 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker { throw RuntimeException("Profile not renamed") } } + + fun launchProfileDeleteTask( + slotId: Int, + portId: Int, + iccid: String + ): Flow? = + launchForegroundTask( + getString(R.string.task_profile_delete), + R.drawable.ic_task_delete + ) { + euiccChannelManager.beginTrackedOperationBlocking(slotId, portId) { + euiccChannelManager.findEuiccChannelByPort( + slotId, + portId + )!!.lpa.deleteProfile(iccid) + + preferenceRepository.notificationDeleteFlow.first() + } + } } \ No newline at end of file diff --git a/app-common/src/main/java/im/angry/openeuicc/ui/ProfileDeleteFragment.kt b/app-common/src/main/java/im/angry/openeuicc/ui/ProfileDeleteFragment.kt index 7586354..467132f 100644 --- a/app-common/src/main/java/im/angry/openeuicc/ui/ProfileDeleteFragment.kt +++ b/app-common/src/main/java/im/angry/openeuicc/ui/ProfileDeleteFragment.kt @@ -3,16 +3,15 @@ package im.angry.openeuicc.ui import android.app.Dialog import android.os.Bundle import android.text.Editable -import android.util.Log import android.widget.EditText import androidx.appcompat.app.AlertDialog import androidx.fragment.app.DialogFragment import androidx.lifecycle.lifecycleScope import im.angry.openeuicc.common.R import im.angry.openeuicc.util.* -import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.launch -import java.lang.Exception class ProfileDeleteFragment : DialogFragment(), EuiccChannelFragmentMarker { companion object { @@ -67,23 +66,23 @@ class ProfileDeleteFragment : DialogFragment(), EuiccChannelFragmentMarker { alertDialog.getButton(AlertDialog.BUTTON_POSITIVE).isEnabled = false alertDialog.getButton(AlertDialog.BUTTON_NEGATIVE).isEnabled = false - lifecycleScope.launch { - try { - doDelete() - } catch (e: Exception) { - Log.d(ProfileDownloadFragment.TAG, "Error deleting profile") - Log.d(ProfileDownloadFragment.TAG, Log.getStackTraceString(e)) - } finally { + requireParentFragment().lifecycleScope.launch { + ensureEuiccChannelManager() + euiccChannelManagerService.waitForForegroundTask() + + euiccChannelManagerService.launchProfileDeleteTask( + slotId, + portId, + requireArguments().getString("iccid")!! + )!!.onStart { if (parentFragment is EuiccProfilesChangedListener) { + // Trigger a refresh in the parent fragment -- it should wait until + // any foreground task is completed before actually doing a refresh (parentFragment as EuiccProfilesChangedListener).onEuiccProfilesChanged() } + dismiss() - } + }.collect() } } - - private suspend fun doDelete() = beginTrackedOperation { - channel.lpa.deleteProfile(requireArguments().getString("iccid")!!) - preferenceRepository.notificationDeleteFlow.first() - } } \ No newline at end of file diff --git a/app-common/src/main/res/drawable/ic_task_delete.xml b/app-common/src/main/res/drawable/ic_task_delete.xml new file mode 100644 index 0000000..883bcaa --- /dev/null +++ b/app-common/src/main/res/drawable/ic_task_delete.xml @@ -0,0 +1,5 @@ + + + + + diff --git a/app-common/src/main/res/values/strings.xml b/app-common/src/main/res/values/strings.xml index bedfe21..cf7ce59 100644 --- a/app-common/src/main/res/values/strings.xml +++ b/app-common/src/main/res/values/strings.xml @@ -36,6 +36,7 @@ Long-running Tasks Downloading eSIM profile Renaming eSIM profile + Deleting eSIM profile New eSIM Server (RSP / SM-DP+) From 48b5f8ce06ac75f664e81374d2bda002a824c1ba Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sun, 29 Sep 2024 16:54:49 -0400 Subject: [PATCH 2/4] Impose timeout on waiting for foreground start --- .../service/EuiccChannelManagerService.kt | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt b/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt index 6061a94..950247c 100644 --- a/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt +++ b/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt @@ -24,6 +24,8 @@ import kotlinx.coroutines.flow.takeWhile import kotlinx.coroutines.flow.transformWhile import kotlinx.coroutines.launch import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout +import kotlinx.coroutines.withTimeoutOrNull import net.typeblog.lpac_jni.ProfileDownloadCallback /** @@ -161,7 +163,19 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker { lifecycleScope.launch(Dispatchers.Main) { // Wait until our self-start command has succeeded. // We can only call startForeground() after that - foregroundStarted.first() + val res = withTimeoutOrNull(30 * 1000) { + foregroundStarted.first() + } + + if (res == null) { + // The only case where the wait above could time out is if the subscriber + // to the flow is stuck. Or we failed to start foreground. + // In that case, we should just set our state back to Idle -- setting it + // to Done wouldn't help much because nothing is going to then set it Idle. + foregroundTaskState.value = ForegroundTaskState.Idle + return@launch + } + updateForegroundNotification(title, iconRes) try { From 653123939c53e9c9d9261e2073810f4c9c251264 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sun, 29 Sep 2024 17:07:15 -0400 Subject: [PATCH 3/4] Set notifications to ongoing --- .../im/angry/openeuicc/service/EuiccChannelManagerService.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt b/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt index 950247c..212194f 100644 --- a/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt +++ b/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt @@ -24,7 +24,6 @@ import kotlinx.coroutines.flow.takeWhile import kotlinx.coroutines.flow.transformWhile import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -import kotlinx.coroutines.withTimeout import kotlinx.coroutines.withTimeoutOrNull import net.typeblog.lpac_jni.ProfileDownloadCallback @@ -119,6 +118,7 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker { .setProgress(100, state.progress, state.progress == 0) .setSmallIcon(iconRes) .setPriority(NotificationCompat.PRIORITY_LOW) + .setOngoing(true) .build() if (state.progress == 0) { From 2721f9127723d3d123109f452e6cb368243f5283 Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Sun, 29 Sep 2024 17:36:58 -0400 Subject: [PATCH 4/4] Make foreground notifications much more reliable --- .../service/EuiccChannelManagerService.kt | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt b/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt index 212194f..6bef221 100644 --- a/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt +++ b/app-common/src/main/java/im/angry/openeuicc/service/EuiccChannelManagerService.kt @@ -25,6 +25,7 @@ import kotlinx.coroutines.flow.transformWhile import kotlinx.coroutines.launch import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeoutOrNull +import kotlinx.coroutines.yield import net.typeblog.lpac_jni.ProfileDownloadCallback /** @@ -102,7 +103,7 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker { } } - private fun updateForegroundNotification(title: String, iconRes: Int) { + private suspend fun updateForegroundNotification(title: String, iconRes: Int) { val channel = NotificationChannelCompat.Builder(CHANNEL_ID, NotificationManagerCompat.IMPORTANCE_LOW) .setName(getString(R.string.task_notification)) @@ -126,6 +127,10 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker { } else if (checkSelfPermission(android.Manifest.permission.POST_NOTIFICATIONS) == PackageManager.PERMISSION_GRANTED) { NotificationManagerCompat.from(this).notify(FOREGROUND_ID, notification) } + + // Yield out so that the main looper can handle the notification event + // Without this yield, the notification sent above will not be shown in time. + yield() } else { stopForeground(STOP_FOREGROUND_REMOVE) } @@ -182,14 +187,13 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker { withContext(Dispatchers.IO) { this@EuiccChannelManagerService.task() } + // This update will be sent by the subscriber (as shown below) foregroundTaskState.value = ForegroundTaskState.Done(null) } catch (t: Throwable) { foregroundTaskState.value = ForegroundTaskState.Done(t) } finally { stopSelf() } - - updateForegroundNotification(title, iconRes) } // We should be the only task running, so we can subscribe to foregroundTaskState @@ -199,20 +203,26 @@ class EuiccChannelManagerService : LifecycleService(), OpenEuiccContextMarker { // it has been completed by that point. return foregroundTaskState.transformWhile { // Also update our notification when we see an update - withContext(Dispatchers.Main) { - updateForegroundNotification(title, iconRes) + // But ignore the first progress = 0 update -- that is the current value. + // we need that to be handled by the main coroutine after it finishes. + if (it !is ForegroundTaskState.InProgress || it.progress != 0) { + withContext(Dispatchers.Main) { + updateForegroundNotification(title, iconRes) + } } emit(it) it !is ForegroundTaskState.Done }.onStart { // When this Flow is started, we unblock the coroutine launched above by // self-starting as a foreground service. - startForegroundService( - Intent( - this@EuiccChannelManagerService, - this@EuiccChannelManagerService::class.java + withContext(Dispatchers.Main) { + startForegroundService( + Intent( + this@EuiccChannelManagerService, + this@EuiccChannelManagerService::class.java + ) ) - ) + } }.onCompletion { foregroundTaskState.value = ForegroundTaskState.Idle } }