Watch

eddy 8ef971c051 Add code review fixes implementation plan

2026-07-26 12:54:11 +02:00

15 KiB

Raw Blame History

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

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

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:

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

New:

    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

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:

    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

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:

        val intf = findCdcDataInterface(device)

with:

        val intf = UsbUtils.findCdcDataInterface(device)

Replace:

        val epOut = findBulkEndpoint(intf, UsbConstants.USB_DIR_OUT)

with:

        val epOut = UsbUtils.findBulkEndpoint(intf, UsbConstants.USB_DIR_OUT)

Replace:

        val epIn = findBulkEndpoint(intf, UsbConstants.USB_DIR_IN)

with:

        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:

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

with:

        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

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:

        registerReceiver(bondReceiver, IntentFilter(BluetoothDevice.ACTION_BOND_STATE_CHANGED))

with:

        registerReceiver(bondReceiver, IntentFilter(BluetoothDevice.ACTION_BOND_STATE_CHANGED), Context.RECEIVER_NOT_EXPORTED)

Step 2: Add SCAN_TIMEOUT_MS constant

In companion object, add:

        private const val SCAN_TIMEOUT_MS = 10000L

Step 3: Add boolean for scan timeout state

Add field:

    private var scanTimedOut = false

Step 4: Fix stopScan to use boolean

Replace stopScan:

    @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:

    @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:

        handler.postDelayed({
            if (isScanning) stopScan()
        }, 10000)

with:

        scanTimedOut = true
        handler.postDelayed({
            if (isScanning) stopScan()
        }, SCAN_TIMEOUT_MS)

And add scanTimedOut = false after isScanning = true:

        isScanning = true
        scanTimedOut = false
        statusText.text = "Scanning..."

Step 6: Add onPause

Add after onResume:

    override fun onPause() {
        super.onPause()
        stopScan()
    }

Step 7: Fix onDestroy — add handler cleanup

Replace onDestroy:

    @SuppressLint("UnspecifiedRegisterReceiverFlag")
    override fun onDestroy() {
        super.onDestroy()
        stopScan()
        bluetoothGatt?.close()
        unregisterReceiver(bondReceiver)
    }

with:

    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

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:

        android:allowBackup="true"

with:

        android:allowBackup="false"

Step 2: Commit

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:

        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:

        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

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

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

15 KiB Raw Blame History

Code Review Fixes Implementation Plan

File Structure

Task 1: Create UsbUtils utility object

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

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

Task 4: Fix SettingsActivity — UsbUtils + thread lifecycle

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

Task 6: Fix AndroidManifest — allowBackup=false

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

Task 8: Final build and install

15 KiB

Raw Blame History