Refactor Improv flow to be more modern and usable in FrontendScreen

app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/FrontendImprovModule.kt

@@ -1,20 +1,37 @@
 package io.homeassistant.companion.android.frontend.improv
 
+import android.content.Context
 import android.content.pm.PackageManager
+import com.wifi.improv.ImprovManager
+import dagger.Binds
 import dagger.Module
 import dagger.Provides
 import dagger.hilt.InstallIn
+import dagger.hilt.android.qualifiers.ApplicationContext
 import dagger.hilt.components.SingletonComponent
+import javax.inject.Singleton
 
 /**
  * Hilt bindings for the frontend's improv (Wi-Fi onboarding for BLE devices) integration.
  */
 @Module
 @InstallIn(SingletonComponent::class)
-object FrontendImprovModule {
+abstract class FrontendImprovModule {
 
-    @Provides
-    fun provideBluetoothCapabilities(packageManager: PackageManager): BluetoothCapabilities = BluetoothCapabilities {
-        packageManager.hasSystemFeature(PackageManager.FEATURE_BLUETOOTH_LE)
+    @Binds
+    @Singleton
+    abstract fun bindImprovRepository(impl: ImprovRepositoryImpl): ImprovRepository
+
+    companion object {
+        @Provides
+        fun provideBluetoothCapabilities(packageManager: PackageManager): BluetoothCapabilities =
+            BluetoothCapabilities {
+                packageManager.hasSystemFeature(PackageManager.FEATURE_BLUETOOTH_LE)
+            }
+
+        @Provides
+        @Singleton
+        fun provideImprovManagerFactory(@ApplicationContext context: Context): ImprovManagerFactory =
+            ImprovManagerFactory { callback -> ImprovManager(context.applicationContext, callback) }
     }
 }

app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/ImprovManagerFactory.kt

@@ -0,0 +1,15 @@
+package io.homeassistant.companion.android.frontend.improv
+
+import com.wifi.improv.ImprovManager
+import com.wifi.improv.ImprovManagerCallback
+
+/**
+ * Creates [ImprovManager] instances on demand for [ImprovRepositoryImpl].
+ *
+ * Exists so the repository does not have to hold an Android `Context` — the factory closes over
+ * the application context at the Hilt provision site, leaving the repository fully unit-testable
+ * with a mock factory.
+ */
+fun interface ImprovManagerFactory {
+    fun create(callback: ImprovManagerCallback): ImprovManager
+}

app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/ImprovRepository.kt

@@ -0,0 +1,50 @@
+package io.homeassistant.companion.android.frontend.improv
+
+import com.wifi.improv.ImprovDevice
+import kotlinx.coroutines.flow.Flow
+
+/**
+ * Repository façade for the [Improv Wi-Fi onboarding protocol](https://www.improv-wifi.com).
+ *
+ * Exposes the protocol as two Flow operations, each fully driven by the collector's lifecycle.
+ *
+ * Both operations require the Android runtime permissions reported by [requiredPermissions];
+ * callers are responsible for requesting them before subscribing.
+ */
+interface ImprovRepository {
+
+    /**
+     * Android runtime permissions required for the BLE scan and the per-device GATT handshake.
+     * The set varies by platform SDK.
+     */
+    val requiredPermissions: List<String>
+
+    /** Whether every entry in [requiredPermissions] are currently granted to the app. */
+    fun hasPermissions(): Boolean
+
+    /**
+     * Emits the cumulative list of devices discovered by the active BLE scan.
+     *
+     * Hot: subscribing starts the scan (or joins one already in flight); the scan tears down
+     * shortly after the last subscriber detaches.
+     */
+    fun scanDevices(): Flow<List<ImprovDevice>>
+
+    /**
+     * Runs the BLE provisioning handshake against [device]:
+     *
+     * 1. Open a GATT connection.
+     * 2. Wait for the device to report it's authorized — some hardware requires a physical
+     *    button press here.
+     * 3. Send [ssid] / [password] over the improv characteristic.
+     * 4. Forward the device's state machine until it reports it has been provisioned.
+     *
+     * Emits a [ProvisioningEvent] for every state transition, error report, and the terminal
+     * [ProvisioningEvent.Provisioned] carrying the integration `domain` the device advertises
+     * (e.g. `"esphome"`) — or `null` when none was reported. The flow completes normally after
+     * that terminal event; cancelling the collector earlier aborts the session.
+     *
+     * Credentials are confined to this call's parameters the repository does not retain them.
+     */
+    fun provisionDevice(device: ImprovDevice, ssid: String, password: String): Flow<ProvisioningEvent>
+}

app/src/main/kotlin/io/homeassistant/companion/android/frontend/improv/ImprovRepositoryImpl.kt

@@ -0,0 +1,217 @@
+package io.homeassistant.companion.android.frontend.improv
+
+import android.Manifest
+import android.annotation.SuppressLint
+import android.os.Build
+import androidx.annotation.VisibleForTesting
+import com.wifi.improv.DeviceState
+import com.wifi.improv.ErrorState
+import com.wifi.improv.ImprovDevice
+import com.wifi.improv.ImprovManager
+import com.wifi.improv.ImprovManagerCallback
+import io.homeassistant.companion.android.common.util.PermissionChecker
+import javax.inject.Inject
+import javax.inject.Singleton
+import kotlinx.coroutines.CoroutineDispatcher
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.NonCancellable
+import kotlinx.coroutines.SupervisorJob
+import kotlinx.coroutines.channels.awaitClose
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.MutableSharedFlow
+import kotlinx.coroutines.flow.MutableStateFlow
+import kotlinx.coroutines.flow.SharedFlow
+import kotlinx.coroutines.flow.SharingStarted
+import kotlinx.coroutines.flow.asStateFlow
+import kotlinx.coroutines.flow.channelFlow
+import kotlinx.coroutines.flow.onCompletion
+import kotlinx.coroutines.flow.onStart
+import kotlinx.coroutines.flow.shareIn
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
+import okhttp3.HttpUrl.Companion.toHttpUrlOrNull
+import timber.log.Timber
+
+/**
+ * Idle window before a refcount-zero `scanDevices()` subscription actually tears down the BLE
+ * scan. Lets brief subscriber gaps (e.g. configuration changes, recomposition) avoid the
+ * start/stop churn.
+ */
+@VisibleForTesting
+internal const val SCAN_IDLE_WINDOW_MS: Long = 500L
+
+@Singleton
+class ImprovRepositoryImpl @VisibleForTesting constructor(
+    private val permissionChecker: PermissionChecker,
+    improvManagerFactory: ImprovManagerFactory,
+    // Test-only override of [Build.VERSION.SDK_INT] so the SDK-branched [requiredPermissions] can
+    // be exercised without Robolectric.
+    private val sdkInt: Int,
+    private val shareInScope: CoroutineScope,
+    // Injected so tests can pin BLE start/stop hops onto the test scheduler.
+    private val backgroundDispatcher: CoroutineDispatcher,
+) : ImprovRepository,
+    ImprovManagerCallback {
+
+    @Inject
+    constructor(permissionChecker: PermissionChecker, improvManagerFactory: ImprovManagerFactory) : this(
+        permissionChecker = permissionChecker,
+        improvManagerFactory = improvManagerFactory,
+        sdkInt = Build.VERSION.SDK_INT,
+        shareInScope = CoroutineScope(SupervisorJob() + Dispatchers.Default),
+        backgroundDispatcher = Dispatchers.IO,
+    )
+
+    private val manager: ImprovManager = improvManagerFactory.create(this)
+
+    private val devices = MutableStateFlow(emptyList<ImprovDevice>())
+    private val stateEvents = MutableSharedFlow<DeviceState>(extraBufferCapacity = 16)
+    private val errorEvents = MutableSharedFlow<ErrorState>(extraBufferCapacity = 16)
+
+    /**
+     * Latest RPC result reported by the library. Read once when [ProvisioningEvent.Provisioned]
+     * is emitted to extract the integration `domain`. Volatile for cross-thread visibility — the
+     * callback may run on the library's internal thread.
+     */
+    @Volatile
+    private var lastRpcResult: List<String> = emptyList()
+
+    private val sharedScanFlow: SharedFlow<List<ImprovDevice>> = devices.asStateFlow()
+        .onStart { startScanInternal() }
+        .onCompletion { stopScanInternal() }
+        .shareIn(shareInScope, SharingStarted.WhileSubscribed(stopTimeoutMillis = SCAN_IDLE_WINDOW_MS), replay = 1)
+
+    override val requiredPermissions: List<String>
+        @SuppressLint("InlinedApi")
+        get() = buildList {
+            add(Manifest.permission.ACCESS_FINE_LOCATION)
+            if (sdkInt >= Build.VERSION_CODES.S) {
+                add(Manifest.permission.BLUETOOTH_SCAN)
+                add(Manifest.permission.BLUETOOTH_CONNECT)
+            } else {
+                add(Manifest.permission.BLUETOOTH)
+                add(Manifest.permission.BLUETOOTH_ADMIN)
+            }
+        }
+
+    override fun hasPermissions(): Boolean = requiredPermissions.all { permissionChecker.hasPermission(it) }
+
+    override fun scanDevices(): Flow<List<ImprovDevice>> = sharedScanFlow
+
+    override fun provisionDevice(device: ImprovDevice, ssid: String, password: String): Flow<ProvisioningEvent> =
+        channelFlow {
+            var credentialsSent = false
+
+            // Forward error events for the duration of the session.
+            val errorJob = launch {
+                errorEvents.collect { error ->
+                    if (error != ErrorState.NO_ERROR) {
+                        send(ProvisioningEvent.ErrorOccurred(error))
+                    }
+                }
+            }
+
+            // Forward state events; drive the AUTHORIZED → sendWifi step; close on PROVISIONED.
+            val stateJob = launch {
+                stateEvents.collect { state ->
+                    send(ProvisioningEvent.StateChanged(state))
+
+                    when (state) {
+                        DeviceState.AUTHORIZED -> if (!credentialsSent) {
+                            try {
+                                manager.sendWifi(ssid, password)
+                                credentialsSent = true
+                            } catch (e: SecurityException) {
+                                Timber.e(e, "Not allowed to send Wi-Fi credentials")
+                                close(e)
+                            }
+                        }
+
+                        DeviceState.PROVISIONED -> {
+                            val domain = lastRpcResult.firstOrNull()
+                                ?.toHttpUrlOrNull()
+                                ?.queryParameter("domain")
+                            send(ProvisioningEvent.Provisioned(domain))
+                            close()
+                        }
+
+                        else -> Unit
+                    }
+                }
+            }
+
+            try {
+                manager.connectToDevice(device)
+            } catch (e: SecurityException) {
+                Timber.e(e, "Not allowed to connect to device")
+                close(e)
+            }
+
+            awaitClose {
+                errorJob.cancel()
+                stateJob.cancel()
+            }
+        }
+
+    // region ImprovManagerCallback
+
+    override fun onConnectionStateChange(device: ImprovDevice?) {
+        if (device == null) {
+            // Disconnect: reset rpc result so a future session starts clean.
+            lastRpcResult = emptyList()
+        }
+    }
+
+    override fun onDeviceFound(device: ImprovDevice) {
+        val current = devices.value
+        if (!current.contains(device)) {
+            devices.tryEmit(current + device)
+        }
+    }
+
+    override fun onErrorStateChange(errorState: ErrorState) {
+        errorEvents.tryEmit(errorState)
+    }
+
+    override fun onScanningStateChange(scanning: Boolean) {
+        // Library scanning state is implicit in subscription to scanDevices(); not re-exposed.
+    }
+
+    override fun onStateChange(state: DeviceState) {
+        stateEvents.tryEmit(state)
+        if (state == DeviceState.PROVISIONED) {
+            // Clear the device list so the next scan starts fresh.
+            devices.tryEmit(emptyList())
+        }
+    }
+
+    override fun onRpcResult(result: List<String>) {
+        lastRpcResult = result
+    }
+
+    // endregion
+
+    private suspend fun startScanInternal() = withContext(backgroundDispatcher) {
+        if (!hasPermissions()) return@withContext
+        try {
+            manager.findDevices()
+        } catch (e: SecurityException) {
+            Timber.e(e, "Not allowed to start scanning")
+        } catch (e: Exception) {
+            Timber.w(e, "Unexpectedly cannot start scanning")
+        }
+    }
+
+    // [NonCancellable] is mandatory: `stopScanInternal()` is invoked from the upstream's
+    // `onCompletion` after shareIn's WhileSubscribed timeout cancels it, so the caller's job is
+    // already cancelled. Without [NonCancellable], `withContext` would short-circuit on
+    // `ensureActive` and the BLE scan would never be torn down.
+    private suspend fun stopScanInternal() = withContext(NonCancellable + backgroundDispatcher) {
+        try {
+            manager.stopScan()
+        } catch (e: Exception) {
+            Timber.w(e, "Cannot stop scanning")
+        }
+    }
+}