# Code Review Fixes Implementation Plan

> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.

**Goal:** Fix all Critical, Important, and selected Minor issues from the code review to make the Android app production-ready.

**Architecture:** Incremental fixes to existing files. New `UsbUtils.kt` utility object to eliminate code duplication. No architectural changes — just bug fixes, lifecycle hardening, and cleanup.

**Tech Stack:** Kotlin, Android SDK API 24-34, USB Host API, BLE API, esp32-flash-lib

---

## File Structure

| File | Action | Responsibility |
|---|---|---|
| `UsbUtils.kt` | **Create** | Shared USB utility functions (findCdcDataInterface, findBulkEndpoint) |
| `MainActivity.kt` | **Modify** | Fix receiver flag, handler cleanup, onPause scan stop, boolean state |
| `SettingsActivity.kt` | **Modify** | Thread lifecycle check, use UsbUtils |
| `EspFlasher.kt` | **Modify** | try/finally connection cleanup, progress mapping, use UsbUtils |
| `UsbSerialTransport.kt` | **Modify** | Throw IOException on read error |
| `AndroidManifest.xml` | **Modify** | allowBackup=false |
| `activity_settings.xml` | **Modify** | Fix download-modus text |

---

### Task 1: Create UsbUtils utility object

**Files:**
- Create: `android/app/src/main/java/com/eddy/atomrelay/UsbUtils.kt`

- [ ] **Step 1: Create UsbUtils.kt**

```kotlin
package com.eddy.atomrelay

import android.hardware.usb.UsbConstants
import android.hardware.usb.UsbDevice
import android.hardware.usb.UsbEndpoint
import android.hardware.usb.UsbInterface

object UsbUtils {
    private const val CDC_DATA_INTERFACE_CLASS = 0x0A

    fun findCdcDataInterface(device: UsbDevice): UsbInterface? {
        for (i in 0 until device.interfaceCount) {
            val intf = device.getInterface(i)
            if (intf.interfaceClass == CDC_DATA_INTERFACE_CLASS) {
                return intf
            }
        }
        return null
    }

    fun findBulkEndpoint(intf: UsbInterface, direction: Int): UsbEndpoint? {
        for (i in 0 until intf.endpointCount) {
            val ep = intf.getEndpoint(i)
            if (ep.type == UsbConstants.USB_ENDPOINT_XFER_BULK && ep.direction == direction) {
                return ep
            }
        }
        return null
    }
}
```

- [ ] **Step 2: Build to verify compilation**

Run: `JAVA_HOME=/home/linuxbrew/.linuxbrew/Cellar/openjdk@21/21.0.12/libexec ./gradlew assembleDebug`
Expected: BUILD SUCCESSFUL

- [ ] **Step 3: Commit**

```bash
git add android/app/src/main/java/com/eddy/atomrelay/UsbUtils.kt
git commit -m "Add UsbUtils utility object for shared USB functions"
```

---

### Task 2: Fix UsbSerialTransport.read() — throw IOException on error

**Files:**
- Modify: `android/app/src/main/java/com/eddy/atomrelay/UsbSerialTransport.kt:15-18`

- [ ] **Step 1: Replace read method**

Old:
```kotlin
    override fun read(buffer: ByteArray, length: Int): Int {
        val n = connection.bulkTransfer(epIn, buffer, length, 5000)
        if (n < 0) return 0
        return n
    }
```

New:
```kotlin
    override fun read(buffer: ByteArray, length: Int): Int {
        val n = connection.bulkTransfer(epIn, buffer, length, 5000)
        if (n < 0) throw IOException("USB read failed")
        return n
    }
```

- [ ] **Step 2: Build to verify**

Run: `JAVA_HOME=/home/linuxbrew/.linuxbrew/Cellar/openjdk@21/21.0.12/libexec ./gradlew assembleDebug`
Expected: BUILD SUCCESSFUL

- [ ] **Step 3: Commit**

```bash
git add android/app/src/main/java/com/eddy/atomrelay/UsbSerialTransport.kt
git commit -m "Fix UsbSerialTransport.read() to throw IOException on error"
```

---

### Task 3: Fix EspFlasher — connection leak + progress mapping + UsbUtils

**Files:**
- Modify: `android/app/src/main/java/com/eddy/atomrelay/EspFlasher.kt`

- [ ] **Step 1: Replace doFlash with try/finally and progress mapping**

Replace the entire `doFlash` method and the `progressCallback` with:

```kotlin
    private fun doFlash(usbManager: UsbManager, device: UsbDevice, callback: FlashCallback) {
        Thread.sleep(500)

        val connection = usbManager.openDevice(device)
            ?: throw IOException("Kan USB-apparaat niet openen")

        try {
            val intf = UsbUtils.findCdcDataInterface(device)
                ?: throw IOException("Geen CDC data interface gevonden")

            if (!connection.claimInterface(intf, true)) {
                throw IOException("Kan USB interface niet claimen")
            }

            val epOut = UsbUtils.findBulkEndpoint(intf, UsbConstants.USB_DIR_OUT)
                ?: throw IOException("Geen USB OUT endpoint gevonden")
            val epIn = UsbUtils.findBulkEndpoint(intf, UsbConstants.USB_DIR_IN)
                ?: throw IOException("Geen USB IN endpoint gevonden")

            postProgress(callback, 0, "Verbinden met ESP bootloader...")

            val transport = UsbSerialTransport(connection, epOut, epIn)

            val assets = context.assets
            val bootloader = assets.open("firmware/bootloader.bin").readBytes()
            val partitions = assets.open("firmware/partitions.bin").readBytes()
            val bootApp0 = assets.open("firmware/boot_app0.bin").readBytes()
            val firmware = assets.open("firmware/firmware.bin").readBytes()

            val totalSize = bootloader.size + partitions.size + bootApp0.size + firmware.size

            data class FlashStep(val name: String, val address: Int, val data: ByteArray, val baseProgress: Int, val range: Int, val compress: Boolean)

            val steps = listOf(
                FlashStep("Bootloader", 0x0, bootloader, 30, 10, false),
                FlashStep("Partitie tabel", 0x8000, partitions, 40, 10, false),
                FlashStep("Boot app0", 0xe000, bootApp0, 50, 10, false),
                FlashStep("Firmware", 0x10000, firmware, 60, 35, true)
            )

            postProgress(callback, 5, "Sync met bootloader...")
            val api = EspFlasherApi.connect(transport)
                .withCallBack(object : ProgressCallback {
                    override fun onProgress(pct: Float) {
                        Log.d(TAG, "Lib progress: $pct%")
                    }
                    override fun onInfo(value: String) {
                        Log.d(TAG, "Info: $value")
                    }
                })

            postProgress(callback, 10, "Chip detecteren...")
            val detected = api.chipDetect()
            val chipId = detected.getChipId()
            Log.i(TAG, "Detected chip: $chipId")
            postProgress(callback, 15, "Chip: ${chipId.readableName}")

            postProgress(callback, 20, "Stub laden...")
            val stub = detected.loadStub()

            postProgress(callback, 25, "Flash wissen...")
            stub.eraseFlash()

            for (step in steps) {
                val sizeStr = if (step.data.size > 1024) "${step.data.size / 1024}KB" else "${step.data.size} bytes"
                postProgress(callback, step.baseProgress, "${step.name} schrijven ($sizeStr)...")
                val writeStub = if (step.compress) stub.withCompression(true) else stub.withCompression(false)
                writeStub.writeFlash(step.address, step.data)
                val written = steps.takeWhile { it !== step }.sumOf { it.data.size } + step.data.size
                val overallProgress = step.baseProgress + (step.range * step.data.size / totalSize).coerceAtLeast(1) + step.range
                postProgress(callback, minOf(step.baseProgress + step.range, 95), "${step.name} geschreven")
            }

            postProgress(callback, 95, "ESP resetten...")
            stub.reset()

            postProgress(callback, 100, "Firmware geflashed!")
            handler.post { callback.onComplete() }
        } finally {
            connection.close()
        }
    }
```

- [ ] **Step 2: Remove the now-duplicated private methods**

Delete `findCdcDataInterface` and `findBulkEndpoint` from EspFlasher.kt (they are now in UsbUtils).

- [ ] **Step 3: Build to verify**

Run: `JAVA_HOME=/home/linuxbrew/.linuxbrew/Cellar/openjdk@21/21.0.12/libexec ./gradlew assembleDebug`
Expected: BUILD SUCCESSFUL

- [ ] **Step 4: Commit**

```bash
git add android/app/src/main/java/com/eddy/atomrelay/EspFlasher.kt
git commit -m "Fix EspFlasher: connection leak, progress mapping, use UsbUtils"
```

---

### Task 4: Fix SettingsActivity — UsbUtils + thread lifecycle

**Files:**
- Modify: `android/app/src/main/java/com/eddy/atomrelay/SettingsActivity.kt`

- [ ] **Step 1: Replace findCdcDataInterface and findBulkEndpoint calls with UsbUtils**

In `readKeyFromEsp`, replace:
```kotlin
        val intf = findCdcDataInterface(device)
```
with:
```kotlin
        val intf = UsbUtils.findCdcDataInterface(device)
```

Replace:
```kotlin
        val epOut = findBulkEndpoint(intf, UsbConstants.USB_DIR_OUT)
```
with:
```kotlin
        val epOut = UsbUtils.findBulkEndpoint(intf, UsbConstants.USB_DIR_OUT)
```

Replace:
```kotlin
        val epIn = findBulkEndpoint(intf, UsbConstants.USB_DIR_IN)
```
with:
```kotlin
        val epIn = UsbUtils.findBulkEndpoint(intf, UsbConstants.USB_DIR_IN)
```

- [ ] **Step 2: Delete the duplicated private methods**

Delete `findCdcDataInterface` and `findBulkEndpoint` from SettingsActivity.kt.

- [ ] **Step 3: Add lifecycle check in thread callback**

In `doKeyExchange`, replace the `Thread` block:

```kotlin
        Thread {
            val result = readKeyFromEsp(usbManager, device)
            handler.post {
```

with:

```kotlin
        Thread {
            val result = readKeyFromEsp(usbManager, device)
            if (isFinishing || isDestroyed) return@Thread
            handler.post {
```

- [ ] **Step 4: Build to verify**

Run: `JAVA_HOME=/home/linuxbrew/.linuxbrew/Cellar/openjdk@21/21.0.12/libexec ./gradlew assembleDebug`
Expected: BUILD SUCCESSFUL

- [ ] **Step 5: Commit**

```bash
git add android/app/src/main/java/com/eddy/atomrelay/SettingsActivity.kt
git commit -m "Fix SettingsActivity: use UsbUtils, add lifecycle guard"
```

---

### Task 5: Fix MainActivity — receiver flag, handler cleanup, onPause, boolean state

**Files:**
- Modify: `android/app/src/main/java/com/eddy/atomrelay/MainActivity.kt`

- [ ] **Step 1: Fix registerReceiver flag**

In `onCreate`, replace:
```kotlin
        registerReceiver(bondReceiver, IntentFilter(BluetoothDevice.ACTION_BOND_STATE_CHANGED))
```
with:
```kotlin
        registerReceiver(bondReceiver, IntentFilter(BluetoothDevice.ACTION_BOND_STATE_CHANGED), Context.RECEIVER_NOT_EXPORTED)
```

- [ ] **Step 2: Add SCAN_TIMEOUT_MS constant**

In companion object, add:
```kotlin
        private const val SCAN_TIMEOUT_MS = 10000L
```

- [ ] **Step 3: Add boolean for scan timeout state**

Add field:
```kotlin
    private var scanTimedOut = false
```

- [ ] **Step 4: Fix stopScan to use boolean**

Replace `stopScan`:
```kotlin
    @SuppressLint("MissingPermission")
    private fun stopScan() {
        if (!isScanning) return
        isScanning = false
        val adapter = BluetoothAdapter.getDefaultAdapter()
        adapter?.bluetoothLeScanner?.stopScan(scanCallback)
        if (statusText.text == "Scanning...") {
            statusText.text = "Device not found"
        }
    }
```
with:
```kotlin
    @SuppressLint("MissingPermission")
    private fun stopScan() {
        if (!isScanning) return
        isScanning = false
        val adapter = BluetoothAdapter.getDefaultAdapter()
        adapter?.bluetoothLeScanner?.stopScan(scanCallback)
        if (scanTimedOut) {
            statusText.text = "Device not found"
        }
    }
```

- [ ] **Step 5: Fix startScan to use constant and boolean**

In `startScan`, replace:
```kotlin
        handler.postDelayed({
            if (isScanning) stopScan()
        }, 10000)
```
with:
```kotlin
        scanTimedOut = true
        handler.postDelayed({
            if (isScanning) stopScan()
        }, SCAN_TIMEOUT_MS)
```

And add `scanTimedOut = false` after `isScanning = true`:
```kotlin
        isScanning = true
        scanTimedOut = false
        statusText.text = "Scanning..."
```

- [ ] **Step 6: Add onPause**

Add after `onResume`:
```kotlin
    override fun onPause() {
        super.onPause()
        stopScan()
    }
```

- [ ] **Step 7: Fix onDestroy — add handler cleanup**

Replace `onDestroy`:
```kotlin
    @SuppressLint("UnspecifiedRegisterReceiverFlag")
    override fun onDestroy() {
        super.onDestroy()
        stopScan()
        bluetoothGatt?.close()
        unregisterReceiver(bondReceiver)
    }
```
with:
```kotlin
    override fun onDestroy() {
        super.onDestroy()
        handler.removeCallbacksAndMessages(null)
        stopScan()
        bluetoothGatt?.close()
        try { unregisterReceiver(bondReceiver) } catch (e: Exception) {}
    }
```

- [ ] **Step 8: Build to verify**

Run: `JAVA_HOME=/home/linuxbrew/.linuxbrew/Cellar/openjdk@21/21.0.12/libexec ./gradlew assembleDebug`
Expected: BUILD SUCCESSFUL

- [ ] **Step 9: Commit**

```bash
git add android/app/src/main/java/com/eddy/atomrelay/MainActivity.kt
git commit -m "Fix MainActivity: receiver flag, handler cleanup, onPause, boolean state"
```

---

### Task 6: Fix AndroidManifest — allowBackup=false

**Files:**
- Modify: `android/app/src/main/AndroidManifest.xml:14`

- [ ] **Step 1: Change allowBackup**

Replace:
```xml
        android:allowBackup="true"
```
with:
```xml
        android:allowBackup="false"
```

- [ ] **Step 2: Commit**

```bash
git add android/app/src/main/AndroidManifest.xml
git commit -m "Disable allowBackup to protect HMAC key from extraction"
```

---

### Task 7: Fix activity_settings.xml — download-modus text

**Files:**
- Modify: `android/app/src/main/res/layout/activity_settings.xml:31`

- [ ] **Step 1: Fix text**

Replace:
```xml
        android:text="Sluit de ESP aan op USB en zet de ESP in download-modus (houd reset 2s in tot de groene LED knippert). Tap dan op 'Firmware Flashen'."
```
with:
```xml
        android:text="Zet de ESP in download-modus (houd reset 2s in tot de groene LED aangaat) en tik op 'Firmware Flashen'."
```

- [ ] **Step 2: Commit**

```bash
git add android/app/src/main/res/layout/activity_settings.xml
git commit -m "Fix download-modus instruction text in settings layout"
```

---

### Task 8: Final build and install

- [ ] **Step 1: Build**

Run: `JAVA_HOME=/home/linuxbrew/.linuxbrew/Cellar/openjdk@21/21.0.12/libexec ./gradlew assembleDebug`
Expected: BUILD SUCCESSFUL

- [ ] **Step 2: Install**

Run: `adb install -r android/app/build/outputs/apk/debug/app-debug.apk`
Expected: Success

- [ ] **Step 3: Commit plan document**

```bash
git add docs/superpowers/plans/2026-07-26-code-review-fixes.md
git commit -m "Add code review fixes implementation plan"
```