Merge pull request #2464 from dv1/combov2-fixes-001

Fixes to combov2 pairing PIN entry UI, the pairing activity, and TDD history retrieval and activity background

pump/combov2/comboctl/src/commonMain/kotlin/info/nightscout/comboctl/main/Pump.kt

@@ -1877,6 +1877,7 @@ class Pump(
     suspend fun fetchTDDHistory() = executeCommand<List<TDDHistoryEntry>>(
         pumpMode = PumpIO.Mode.REMOTE_TERMINAL,
         isIdempotent = true,
+        allowExecutionWhileSuspended = true,
         description = FetchingTDDHistoryCommandDesc()
     ) {
         tddHistoryProgressReporter.reset(Unit)

pump/combov2/src/main/kotlin/info/nightscout/pump/combov2/activities/ComboV2PairingActivity.kt

@@ -1,11 +1,17 @@
 package info.nightscout.pump.combov2.activities
 
+import android.Manifest
 import android.app.Activity
+import android.content.pm.PackageManager
+import android.os.Build
 import android.os.Bundle
 import android.text.Editable
 import android.text.TextWatcher
 import android.view.View
+import androidx.activity.ComponentActivity
+import androidx.activity.result.ActivityResultLauncher
 import androidx.activity.result.contract.ActivityResultContracts
+import androidx.core.content.ContextCompat
 import androidx.databinding.DataBindingUtil
 import androidx.lifecycle.Lifecycle
 import androidx.lifecycle.lifecycleScope
@@ -20,42 +26,65 @@ import info.nightscout.pump.combov2.databinding.Combov2PairingActivityBinding
 import info.nightscout.rx.logging.AAPSLogger
 import info.nightscout.rx.logging.LTag
 import info.nightscout.shared.interfaces.ResourceHelper
+import kotlinx.coroutines.CompletableJob
+import kotlinx.coroutines.Job
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import javax.inject.Inject
 
+// A counterpart to BlePreCheckImpl that is designed for coroutines.
+private class BluetoothPermissionChecks(
+    private val activity: ComponentActivity,
+    private val permissions: List<String>,
+    private val aapsLogger: AAPSLogger
+) {
+    private val activityResultLauncher: ActivityResultLauncher<Array<String>>
+    private var waitForCompletion: CompletableJob? = null
+
+    init {
+        activityResultLauncher = activity.registerForActivityResult(ActivityResultContracts.RequestMultiplePermissions()) {
+            waitForCompletion?.complete()
+        }
+    }
+
+    suspend fun requestAndCheck() {
+        val missingPermissions = permissions
+            .filter {
+                ContextCompat.checkSelfPermission(activity, it) != PackageManager.PERMISSION_GRANTED
+            }
+            .toTypedArray()
+
+        if (missingPermissions.isEmpty())
+            return
+
+        aapsLogger.debug(LTag.PUMP, "Missing permissions: " + missingPermissions.joinToString(", "))
+
+        waitForCompletion = Job()
+        activityResultLauncher.launch(missingPermissions)
+        waitForCompletion?.join()
+        waitForCompletion = null
+    }
+
+    fun unregister() {
+        activityResultLauncher.unregister()
+    }
+}
+
 class ComboV2PairingActivity : DaggerAppCompatActivity() {
 
     @Inject lateinit var aapsLogger: AAPSLogger
     @Inject lateinit var rh: ResourceHelper
     @Inject lateinit var combov2Plugin: ComboV2Plugin
 
+    private var uiInitialized = false
+    private var unregisterActivityLauncher = {}
+    private var bluetoothPermissionChecks: BluetoothPermissionChecks? = null
+
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
 
-        val binding: Combov2PairingActivityBinding = DataBindingUtil.setContentView(
-            this, R.layout.combov2_pairing_activity)
-
-        // In the NotInitialized state, the PumpManager is unavailable because it cannot
-        // function without Bluetooth permissions. Several of ComboV2Plugin's functions
-        // such as getPairingProgressFlow() depend on PumpManager though. To prevent UI
-        // controls from becoming active without having a PumpManager, show instead a
-        // view on the activity that explains why pairing is currently not possible.
-        if (combov2Plugin.driverStateUIFlow.value == ComboV2Plugin.DriverState.NotInitialized) {
-            aapsLogger.info(LTag.PUMP, "Cannot pair right now; disabling pairing UI controls, showing message instead")
-
-            binding.combov2PairingSectionInitial.visibility = View.GONE
-            binding.combov2PairingSectionCannotPairDriverNotInitialized.visibility = View.VISIBLE
-
-            binding.combov2CannotPairGoBack.setOnClickListener {
-                finish()
-            }
-
-            return
-        }
-
         // Install an activity result caller for when the user presses
         // "deny" or "reject" in the dialog that pops up when Android
         // asks for permission to enable device discovery. In such a
@@ -74,7 +103,131 @@ class ComboV2PairingActivity : DaggerAppCompatActivity() {
         combov2Plugin.customDiscoveryActivityStartCallback = { intent ->
             startPairingActivityLauncher.launch(intent)
         }
+        unregisterActivityLauncher = {
+            startPairingActivityLauncher.unregister()
+        }
 
+        val binding: Combov2PairingActivityBinding = DataBindingUtil.setContentView(
+            this, R.layout.combov2_pairing_activity)
+
+        val thisActivity = this
+
+        // Set the pairing sections to initially show the "not initialized" one
+        // in case the Bluetooth permissions haven't been granted yet by the user.
+        binding.combov2PairingSectionInitial.visibility = View.GONE
+        binding.combov2PairingSectionCannotPairDriverNotInitialized.visibility = View.VISIBLE
+
+        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) {
+            // Launch the BluetoothPermissionChecks in the CREATED lifecycle state.
+            // This is important, because registering an activity (which the
+            // BluetoothPermissionChecks class does) must take place _before_ the
+            // STARTED state is reached.
+            lifecycleScope.launch {
+                lifecycle.repeatOnLifecycle(Lifecycle.State.CREATED) {
+                    aapsLogger.debug(LTag.PUMP, "Creating and registering BT permissions check object")
+                    bluetoothPermissionChecks = BluetoothPermissionChecks(
+                        thisActivity,
+                        listOf(
+                            Manifest.permission.BLUETOOTH_SCAN,
+                            Manifest.permission.BLUETOOTH_CONNECT
+                        ),
+                        aapsLogger
+                    )
+                }
+            }
+
+            // Unregister any activity that BluetoothPermissionChecks previously
+            // registered if this pairing activity is getting destroyed.
+            lifecycleScope.launch {
+                lifecycle.repeatOnLifecycle(Lifecycle.State.DESTROYED) {
+                    aapsLogger.debug(LTag.PUMP, "Unregistering BT permissions check object")
+                    bluetoothPermissionChecks?.unregister()
+                    bluetoothPermissionChecks = null
+                }
+            }
+        }
+
+        lifecycleScope.launch {
+            lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) {
+                bluetoothPermissionChecks?.let {
+                    aapsLogger.debug(LTag.PUMP, "Requesting and checking BT permissions")
+                    it.requestAndCheck()
+                }
+                combov2Plugin.driverStateUIFlow
+                    .onEach { driverState ->
+                        if (!uiInitialized) {
+                            when (driverState) {
+                                // In the NotInitialized state, the PumpManager is unavailable because it cannot
+                                // function without Bluetooth permissions. Several of ComboV2Plugin's functions
+                                // such as getPairingProgressFlow() depend on PumpManager though. To prevent UI
+                                // controls from becoming active without having a PumpManager, show instead a
+                                // view on the activity that explains why pairing is currently not possible.
+                                ComboV2Plugin.DriverState.NotInitialized -> {
+                                    aapsLogger.info(LTag.PUMP, "Cannot pair right now; disabling pairing UI controls, showing message instead")
+
+                                    binding.combov2PairingSectionInitial.visibility = View.GONE
+                                    binding.combov2PairingSectionCannotPairDriverNotInitialized.visibility = View.VISIBLE
+
+                                    binding.combov2CannotPairGoBack.setOnClickListener {
+                                        finish()
+                                    }
+                                }
+
+                                else -> {
+                                    binding.combov2PairingSectionCannotPairDriverNotInitialized.visibility = View.GONE
+                                    setupUi(binding)
+                                    uiInitialized = true
+                                }
+                            }
+                        }
+                    }
+                    .launchIn(this)
+            }
+        }
+    }
+
+    override fun onBackPressed() {
+        aapsLogger.info(LTag.PUMP, "User pressed the back button; cancelling any ongoing pairing")
+        combov2Plugin.cancelPairing()
+        @Suppress("DEPRECATION")
+        super.onBackPressed()
+    }
+
+    override fun onDestroy() {
+        // In the NotInitialized state, getPairingProgressFlow() crashes because there
+        // is no PumpManager present. But in that state, the pairing progress flow needs
+        // no reset because no pairing can happen in that state anyway.
+        if (combov2Plugin.driverStateUIFlow.value != ComboV2Plugin.DriverState.NotInitialized) {
+            // Reset the pairing progress reported to allow for future pairing attempts.
+            // Do this only after pairing was finished or aborted. onDestroy() can be
+            // called in the middle of a pairing process, and we do not want to reset
+            // the progress reporter mid-pairing.
+            when (combov2Plugin.getPairingProgressFlow().value.stage) {
+                BasicProgressStage.Finished,
+                is BasicProgressStage.Aborted -> {
+                    aapsLogger.debug(
+                        LTag.PUMP,
+                        "Resetting pairing progress reporter after pairing was finished/aborted"
+                    )
+                    combov2Plugin.resetPairingProgress()
+                }
+
+                else                          -> Unit
+            }
+        }
+
+        // Remove the activity start callback and unregister the activity
+        // launcher to make sure that future registerForActivityResult()
+        // calls start from a blank slate. (This is about the discovery
+        // activity, not about the BluetoothPermissionChecks ones.)
+        combov2Plugin.customDiscoveryActivityStartCallback = null
+        unregisterActivityLauncher.invoke()
+        unregisterActivityLauncher = {}
+
+        super.onDestroy()
+    }
+
+    private fun setupUi(binding: Combov2PairingActivityBinding) {
         binding.combov2PairingFinishedOk.setOnClickListener {
             finish()
         }
@@ -197,112 +350,75 @@ class ComboV2PairingActivity : DaggerAppCompatActivity() {
             })
         }
 
-        lifecycleScope.launch {
+        combov2Plugin.getPairingProgressFlow()
-            lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) {
+            .onEach { progressReport ->
-                combov2Plugin.getPairingProgressFlow()
+                val stage = progressReport.stage
-                    .onEach { progressReport ->
-                        val stage = progressReport.stage
 
-                        binding.combov2PairingSectionInitial.visibility =
+                binding.combov2PairingSectionInitial.visibility =
-                            if (stage == BasicProgressStage.Idle) View.VISIBLE else View.GONE
+                    if (stage == BasicProgressStage.Idle) View.VISIBLE else View.GONE
-                        binding.combov2PairingSectionFinished.visibility =
+                binding.combov2PairingSectionFinished.visibility =
-                            if (stage == BasicProgressStage.Finished) View.VISIBLE else View.GONE
+                    if (stage == BasicProgressStage.Finished) View.VISIBLE else View.GONE
-                        binding.combov2PairingSectionAborted.visibility =
+                binding.combov2PairingSectionAborted.visibility =
-                            if (stage is BasicProgressStage.Aborted) View.VISIBLE else View.GONE
+                    if (stage is BasicProgressStage.Aborted) View.VISIBLE else View.GONE
-                        binding.combov2PairingSectionMain.visibility = when (stage) {
+                binding.combov2PairingSectionMain.visibility = when (stage) {
-                            BasicProgressStage.Idle,
+                    BasicProgressStage.Idle,
-                            BasicProgressStage.Finished,
+                    BasicProgressStage.Finished,
-                            is BasicProgressStage.Aborted -> View.GONE
+                    is BasicProgressStage.Aborted -> View.GONE
-                            else -> View.VISIBLE
+                    else -> View.VISIBLE
-                        }
-
-                        if (stage is BasicProgressStage.Aborted) {
-                            binding.combov2PairingAbortedReasonText.text = when (stage) {
-                                is BasicProgressStage.Cancelled -> rh.gs(R.string.combov2_pairing_cancelled)
-                                is BasicProgressStage.Timeout -> rh.gs(R.string.combov2_pairing_combo_scan_timeout_reached)
-                                is BasicProgressStage.Error -> rh.gs(R.string.combov2_pairing_failed_due_to_error, stage.cause.toString())
-                                else -> rh.gs(R.string.combov2_pairing_aborted_unknown_reasons)
-                            }
-                        }
-
-                        binding.combov2CurrentPairingStepDesc.text = when (val progStage = stage) {
-                            BasicProgressStage.ScanningForPumpStage ->
-                                rh.gs(R.string.combov2_scanning_for_pump)
-
-                            is BasicProgressStage.EstablishingBtConnection -> {
-                                rh.gs(
-                                    R.string.combov2_establishing_bt_connection,
-                                    progStage.currentAttemptNr
-                                )
-                            }
-
-                            BasicProgressStage.PerformingConnectionHandshake ->
-                                rh.gs(R.string.combov2_pairing_performing_handshake)
-
-                            BasicProgressStage.ComboPairingKeyAndPinRequested  ->
-                                rh.gs(R.string.combov2_pairing_pump_requests_pin)
-
-                            BasicProgressStage.ComboPairingFinishing ->
-                                rh.gs(R.string.combov2_pairing_finishing)
-
-                            else -> ""
-                        }
-
-                        if (stage == BasicProgressStage.ComboPairingKeyAndPinRequested) {
-                            binding.combov2PinEntryUi.visibility = View.VISIBLE
-                        } else
-                            binding.combov2PinEntryUi.visibility = View.GONE
-
-                        // Scanning for the pump can take a long time and happens at the
-                        // beginning, so set the progress bar to indeterminate during that
-                        // time to show _something_ to the user.
-                        binding.combov2PairingProgressBar.isIndeterminate =
-                            (stage == BasicProgressStage.ScanningForPumpStage)
-
-                        binding.combov2PairingProgressBar.progress = (progressReport.overallProgress * 100).toInt()
-                    }
-                    .launchIn(this)
-
-                combov2Plugin.previousPairingAttemptFailedFlow
-                    .onEach { previousAttemptFailed ->
-                        binding.combov2PinFailureIndicator.visibility =
-                            if (previousAttemptFailed) View.VISIBLE else View.GONE
-                    }
-                    .launchIn(this)
-            }
-        }
-    }
-
-    override fun onBackPressed() {
-        aapsLogger.info(LTag.PUMP, "User pressed the back button; cancelling any ongoing pairing")
-        combov2Plugin.cancelPairing()
-        @Suppress("DEPRECATION")
-        super.onBackPressed()
-    }
-
-    override fun onDestroy() {
-        // In the NotInitialized state, getPairingProgressFlow() crashes because there
-        // is no PumpManager present. But in that state, the pairing progress flow needs
-        // no reset because no pairing can happen in that state anyway.
-        if (combov2Plugin.driverStateUIFlow.value != ComboV2Plugin.DriverState.NotInitialized) {
-            // Reset the pairing progress reported to allow for future pairing attempts.
-            // Do this only after pairing was finished or aborted. onDestroy() can be
-            // called in the middle of a pairing process, and we do not want to reset
-            // the progress reporter mid-pairing.
-            when (combov2Plugin.getPairingProgressFlow().value.stage) {
-                BasicProgressStage.Finished,
-                is BasicProgressStage.Aborted -> {
-                    aapsLogger.debug(
-                        LTag.PUMP,
-                        "Resetting pairing progress reporter after pairing was finished/aborted"
-                    )
-                    combov2Plugin.resetPairingProgress()
                 }
 
-                else                          -> Unit
+                if (stage is BasicProgressStage.Aborted) {
-            }
+                    binding.combov2PairingAbortedReasonText.text = when (stage) {
-        }
+                        is BasicProgressStage.Cancelled -> rh.gs(R.string.combov2_pairing_cancelled)
+                        is BasicProgressStage.Timeout -> rh.gs(R.string.combov2_pairing_combo_scan_timeout_reached)
+                        is BasicProgressStage.Error -> rh.gs(R.string.combov2_pairing_failed_due_to_error, stage.cause.toString())
+                        else -> rh.gs(R.string.combov2_pairing_aborted_unknown_reasons)
+                    }
+                }
 
-        super.onDestroy()
+                binding.combov2CurrentPairingStepDesc.text = when (val progStage = stage) {
+                    BasicProgressStage.ScanningForPumpStage ->
+                        rh.gs(R.string.combov2_scanning_for_pump)
+
+                    is BasicProgressStage.EstablishingBtConnection -> {
+                        rh.gs(
+                            R.string.combov2_establishing_bt_connection,
+                            progStage.currentAttemptNr
+                        )
+                    }
+
+                    BasicProgressStage.PerformingConnectionHandshake ->
+                        rh.gs(R.string.combov2_pairing_performing_handshake)
+
+                    BasicProgressStage.ComboPairingKeyAndPinRequested  ->
+                        rh.gs(R.string.combov2_pairing_pump_requests_pin)
+
+                    BasicProgressStage.ComboPairingFinishing ->
+                        rh.gs(R.string.combov2_pairing_finishing)
+
+                    else -> ""
+                }
+
+                if (stage == BasicProgressStage.ComboPairingKeyAndPinRequested) {
+                    binding.combov2PinEntryUi.visibility = View.VISIBLE
+                } else
+                    binding.combov2PinEntryUi.visibility = View.GONE
+
+                // Scanning for the pump can take a long time and happens at the
+                // beginning, so set the progress bar to indeterminate during that
+                // time to show _something_ to the user.
+                binding.combov2PairingProgressBar.isIndeterminate =
+                    (stage == BasicProgressStage.ScanningForPumpStage)
+
+                binding.combov2PairingProgressBar.progress = (progressReport.overallProgress * 100).toInt()
+            }
+            .launchIn(lifecycleScope)
+
+        combov2Plugin.previousPairingAttemptFailedFlow
+            .onEach { previousAttemptFailed ->
+                binding.combov2PinFailureIndicator.visibility =
+                    if (previousAttemptFailed) View.VISIBLE else View.GONE
+            }
+            .launchIn(lifecycleScope)
     }
 }

pump/combov2/src/main/res/layout/combov2_pairing_activity.xml

@@ -189,6 +189,8 @@
                     android:id="@+id/combov2_pin_entry_ui"
                     android:layout_width="match_parent"
                     android:layout_height="wrap_content"
+                    android:layout_marginLeft="16dp"
+                    android:layout_marginRight="16dp"
                     android:layout_marginBottom="20dp"
                     android:gravity="center_horizontal"
                     android:orientation="vertical"
@@ -210,7 +212,8 @@
 
                         <EditText
                             android:id="@+id/combov2_pin_entry_edit"
-                            android:layout_width="200dp"
+                            android:layout_weight="1"
+                            android:layout_width="0dp"
                             android:layout_height="wrap_content"
                             android:autofillHints="@string/combov2_pin_entry_hint"
                             android:digits="0123456789 "

ui/src/main/AndroidManifest.xml

@@ -26,7 +26,8 @@
 
         <activity
             android:name=".activities.TDDStatsActivity"
-            android:exported="false" />
+            android:exported="false"
+            android:theme="@style/AppTheme.NoActionBar" />
         <activity
             android:name="info.nightscout.ui.activities.BolusProgressHelperActivity"
             android:exported="false"