chore: remove research docs from git tracking, add to .gitignore
This commit is contained in:
eddy
parent
0833327cea
commit
15498d025a
14 changed files with 9 additions and 2778 deletions
|
|
@ -1,499 +0,0 @@
|
|||
# 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"
|
||||
```
|
||||
Loading…
Reference in a new issue