Skip to content

Conversation

@torlando-tech
Copy link
Owner

Summary

  • Add complete RNode firmware flashing capability with 5-step wizard UI
  • Implement core flasher protocols: Nordic DFU (nRF52) and esptool (ESP32)
  • Support auto-detection of board type and firmware version
  • Download firmware directly from GitHub releases with caching

Features

Core Flasher Implementation (reticulum module)

  • RNodeFlasher: Main orchestration with USB/serial communication
  • NordicDFUFlasher: DFU protocol for nRF52-based devices (RAK4631)
  • ESPToolFlasher: esptool protocol for ESP32 devices (T-Beam, Heltec, T-Deck)
  • FirmwareDownloader: Download firmware from GitHub releases
  • FirmwarePackage: Parse and manage firmware ZIP packages
  • RNodeDetector: Auto-detect board type and firmware version
  • SLIP/KISS codecs: Serial communication framing

UI Implementation (5-step wizard)

  1. Device Selection - List USB devices, request permissions
  2. Device Detection - Auto-detect board/firmware or manual select
  3. Firmware Selection - Choose board, frequency band, version with download
  4. Flash Progress - Determinate progress bar with cancel confirmation
  5. Complete - Success/failure states with next actions (Configure RNode, Flash Another)

Entry Point

Settings → RNodeFlasherCard → Open Flasher → Wizard

Test plan

  • Unit tests for codec implementations (CRC16, SLIP, KISS)
  • Unit tests for FirmwarePackage parsing
  • Manual test: Full wizard flow with RAK4631 via USB
  • Manual test: Full wizard flow with ESP32 device (T-Beam, Heltec)
  • Manual test: Cancel during flash
  • Manual test: USB disconnect recovery
  • Manual test: Permission denied flow

Screenshots

To be added after manual testing

🤖 Generated with Claude Code

@torlando-tech torlando-tech linked an issue Jan 20, 2026 that may be closed by this pull request
@torlando-tech torlando-tech force-pushed the feature/rnode-flasher branch 2 times, most recently from b157cd7 to 7477072 Compare January 25, 2026 04:28
Moved v0.7.3-MILESTONE-AUDIT.md to milestones/ archive folder.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@torlando-tech torlando-tech added this to the v0.8.0 milestone Jan 29, 2026
torlando-tech and others added 23 commits January 28, 2026 22:05
Implement complete RNode firmware flashing capability with:

Core flasher implementation (reticulum module):
- RNodeFlasher: Main orchestration with USB/serial communication
- NordicDFUFlasher: DFU protocol for nRF52-based devices (RAK4631)
- ESPToolFlasher: esptool protocol for ESP32 devices (T-Beam, Heltec)
- FirmwareDownloader: Download firmware from GitHub releases
- FirmwarePackage: Parse and manage firmware ZIP packages
- RNodeDetector: Auto-detect board type and firmware version
- SLIP/KISS codecs for serial communication framing

UI implementation (5-step wizard):
- Step 1: Device Selection - list USB devices, request permissions
- Step 2: Device Detection - auto-detect board/firmware or manual select
- Step 3: Firmware Selection - choose board, band, version with download
- Step 4: Flash Progress - determinate progress with cancel confirmation
- Step 5: Complete - success/failure states with next actions

Entry point from Settings via RNodeFlasherCard.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Some RNode firmware files (e.g., rnode_firmware_tbeam_supreme.zip)
support multiple frequency bands configured at runtime, so they
don't have a _868 or _433 suffix in the filename.

Update findFirmwareAsset() to:
1. First try to find a band-specific firmware file
2. Fall back to unified firmware (exact match: {prefix}.zip)

This fixes "No firmware found for LilyGO T-Beam Supreme" error.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
ESP32-S3 flashing fixes:
- Add OFFSET_BOOTLOADER_ESP32_S3 = 0x0 (vs 0x1000 for standard ESP32)
- Add isEsp32S3() to detect ESP32-S3 boards (T-Beam Supreme, T-Deck,
  Heltec v3/v4)
- Pass board to ESPToolFlasher.flash() to use correct bootloader offset
- Log which chip variant and offset is being used

Manual board selection fixes:
- Allow proceeding from detection step when manual selection is enabled
  (board is selected in the next step)
- This fixes the case where EEPROM is wiped and device shows as "unknown"

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When a device is detected but the board type is UNKNOWN (e.g., after
EEPROM wipe), show a new "UnknownBoardState" UI that:
- Indicates the device was detected but board is unknown
- Shows available device info (platform, MCU, firmware version)
- Provides "Continue with Manual Selection" button

This handles the case where detection succeeds but returns board=UNKNOWN
because the EEPROM provisioning data was wiped.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When a device is detected with board type UNKNOWN (e.g., EEPROM wiped),
automatically set useManualBoardSelection=true and selectedBoard=null.
This ensures step 3 (Firmware Selection) always shows the board type
dropdown for unknown devices, rather than requiring the user to click
"Continue with Manual Selection" first.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add support for flashing ESP32-S3 devices with native USB-JTAG-Serial
(VID 0x303A, PID 0x1001) such as Heltec LoRa32 V4. These devices can
now be flashed even when they have factory firmware (not RNode).

Key changes:
- Add raw mode to KotlinUSBBridge that stops SerialInputOutputManager
  so readBlocking() can receive bootloader responses
- Use Future.cancel(true) to forcefully interrupt the ioManager thread
- Add bootloader mode toggle in UI for fresh devices that are already
  in bootloader mode (hold PRG + tap RST)
- Add dynamic timeout calculation for flash erase (10s/MB, min 10s)
- Implement proper DTR/RTS bootloader entry sequence for native USB
- Add FLASH_END command with reboot flag (auto-reboot still requires
  manual reset on ESP32-S3 native USB)

Note: Auto-reboot after flashing does not work on ESP32-S3 native USB
devices - user must press RST button after flashing completes.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
After flashing firmware to fresh devices, they need to be provisioned
with device identity information and have their firmware hash set.

Changes:
- Add EEPROM provisioning to RNodeDetector (writeEeprom, provisionDevice,
  setFirmwareHash, getFirmwareHash, provisionAndSetFirmwareHash)
- Add NeedsManualReset and Provisioning states to flash workflow
- For ESP32-S3 native USB devices, show manual reset instructions after
  flashing completes (auto-reboot doesn't work reliably)
- User clicks "I've Reset the Device" to trigger provisioning step
- Provisioning writes: product, model, hwRev, serial, timestamp,
  MD5 checksum, blank signature (128 zeros), and lock byte
- After provisioning, firmware hash is read from device and set

This follows the same flow as steps 3-4 in the RNode web flasher:
3. Provision EEPROM (device info, checksum, blank signature)
4. Set Firmware Hash (read from device and write back)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ption

- Add retry logic when connecting for provisioning (up to 5 attempts)
- If original device ID fails, scan for native USB devices by VID/PID
- Add "Provision Only (Skip Flashing)" button to firmware selection step
  for testing or when firmware was flashed externally
- Show connection attempt number in provisioning UI

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The RNode firmware doesn't calculate its own hash on boot (returns zeros).
Instead, we now calculate the SHA256 hash from the firmware binary file
and use that during provisioning.

Changes:
- Add calculateFirmwareBinaryHash() to FirmwarePackage to extract and hash
  the application binary from the firmware ZIP
- Store firmware hash in FlashState.NeedsManualReset and pass through
  provisioning flow
- Update provisionAndSetFirmwareHash() to accept optional pre-calculated hash
- Update FlasherViewModel to store and pass firmware hash for both
  flash+provision and provision-only flows

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The 'Missing Config' display is expected and correct for devices used
with Reticulum apps (Columba, Sideband, MeshChat). These apps send radio
parameters at runtime via KISS commands (Normal/host-controlled mode).

TNC mode (saved radio config) is only needed for standalone KISS TNC
operation with amateur radio software and should NOT be enabled for
Reticulum use.

Changes:
- Remove writeDefaultRadioConfig() call from provisioning flow
- Rename function to enableTncMode() with customizable parameters
- Add disableTncMode() function for returning to normal mode
- Update comments explaining 'Missing Config' is normal behavior
- Keep frequency band selection for EEPROM model code

This matches behavior of rnodeconf --autoinstall and web rnode-flasher.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Auto-formatted code to pass ktlint checks after rebasing onto main.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…l devices

Two issues fixed:

1. Firmware hash calculation was wrong for ESP32 devices:
   - RNode firmware embeds SHA256 hash in last 32 bytes of binary
   - Correct: SHA256(firmwareData[0:-32]) == embeddedHash
   - Was: SHA256(entire binary) - always wrong

2. Provisioning was skipped for non-native USB devices:
   - Only ESP32-S3 native USB devices went through provisioning
   - Devices with USB-UART bridges just flashed without EEPROM write
   - Now: All devices get provisioned after successful flash

Also includes:
- Detekt issue fixes (suppressions, refactoring, naming)
- Locale.ROOT for String.format calls
- Test updates for camelCase parameter names
- Increased post-flash delay from 2s to 5s for proper reboot

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add bandExplicitlySelected flag to enforce user confirmation of MHz band
- Show visual warning (red highlight) when band not yet selected
- Require either cached firmware OR version selection before proceeding
- Filter available versions to exclude already-cached firmware
- Clear selections when board/band changes to prevent stale state
- Add "Will download" card when version is queued for download

Fixes issue where users could proceed without selecting frequency band,
causing duplicate firmware downloads for wrong band.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…lReset

The bootloader sync was failing on ESP32-S3 native USB devices because
our reset sequence differed from esptool's USBJTAGSerialReset pattern.

Key changes:
- Match esptool's exact DTR/RTS transition pattern: after entering reset
  (RTS=true), immediately release DTR then set RTS again (Windows
  workaround for usbser.sys driver)
- Use 100ms delays instead of 200ms to match esptool timing
- Add boot log detection after reset to look for "waiting for download"
- Flush input before each sync attempt and reduce retry delay to 50ms

The USB-JTAG-Serial peripheral triggers download mode based on the
pattern of DTR/RTS transitions, not by sampling IO0 state like classic
ESP32 with USB-UART bridge.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
With the reset sequence now matching esptool's USBJTAGSerialReset, native
USB devices reboot reliably after flashing. Remove the special case that
required manual reset and let native USB devices use the same automatic
provisioning flow as other devices.

The provisionDevice() function already handles USB re-enumeration with
up to 5 retries, so it works seamlessly when the device reconnects after
reboot. Native USB devices get 6 seconds instead of 5 to allow for USB
re-enumeration time.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…type

The reset sequence should be chosen based on the USB interface type
(native USB-JTAG-Serial vs USB-UART bridge), not the chip type (S3 vs
classic ESP32).

- Heltec V3: ESP32-S3 chip but uses CH340 USB-UART bridge → ClassicReset
- T-Beam Supreme: ESP32-S3 chip with native USB → USBJTAGSerialReset
- Heltec V4: ESP32-S3 chip with native USB → USBJTAGSerialReset

This matches esptool's behavior with --before default_reset, which
auto-detects based on USB VID/PID (0x303A:0x1001 = native USB).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add FLAG_KEEP_SCREEN_ON to prevent the screen from turning off during
the flashing process, which can take several minutes. The flag is set
when FlashProgressStep enters composition and cleared when leaving.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The baud rate switch logic was checking chip type (isS3) instead of USB
interface type (isNativeUsb). This caused ESP32-S3 devices with USB-UART
bridges (like Heltec V3 with CH340) to stay at 115200 baud instead of
switching to 921600.

- Native USB (VID 0x303A): baud rate doesn't apply (not UART)
- USB-UART bridges (CH340, CP2102, etc.): can use 921600 baud

This significantly speeds up flashing for devices like Heltec V3.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add 79 new unit tests covering the flasher changes:

ESPToolFlasherTest (46 tests):
- isNativeUsbDevice() for ESP32-S3 native USB detection
- VID/PID detection for CH340, CP2102, FTDI bridges
- isEsp32S3() for all board types
- Bootloader offset calculation (0x0 for S3, 0x1000 for others)
- Flash memory offset constants
- Critical distinction: USB interface type vs chip type

NordicDFUFlasherTest (13 tests):
- nRF52 board platform detection
- DFU mode requirements for nRF52 boards
- ESPTool vs DFU flasher selection
- Baud rate constants (1200 touch, 115200 flash, 921600 ESPTool)
- Firmware prefix patterns

RNodeFlasherStateTest (20 tests):
- All FlashState variants coverage
- Equality and hashcode implementations
- NeedsManualReset firmware hash handling
- Property access verification

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When error messages are long (like the "manual bootloader entry"
message for ESP32-S3 devices), the content could push the action
buttons behind the navigation bar.

Changes:
- Add verticalScroll to FailureContent composable
- Add navigationBarsPadding to prevent content from being hidden
- Add bottom padding for extra spacing before buttons

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The manual bootloader mode toggle is no longer needed after the reset
sequence fixes that properly handle ESP32-S3 native USB detection.
The flasher now correctly enters bootloader mode automatically based
on USB VID/PID detection.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When an unconfigured USB device is connected, show an intermediate
screen with two options:
- Flash Firmware: navigate to the RNode Flasher
- Configure RNode: navigate to the RNode Configuration Wizard

This gives users a clear choice instead of assuming they always want
to configure an already-flashed device.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Tests cover equality, hashcode, and property access for:
- UsbDeviceAction (new)
- RNodeWizardWithUsb
- DirectFlash
- InterfaceStats
- Type hierarchy verification

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@torlando-tech torlando-tech marked this pull request as ready for review January 29, 2026 05:40
@sentry
Copy link

sentry bot commented Jan 29, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 29, 2026

Greptile Overview

Greptile Summary

This PR implements a comprehensive RNode firmware flashing feature with a 5-step wizard UI that enables users to flash firmware to RNode devices via USB. The implementation includes support for both nRF52-based devices (using Nordic DFU protocol) and ESP32-based devices (using ESPTool protocol).

Key Changes

Core Flasher Implementation (reticulum module)

  • RNodeFlasher: Main orchestration class that auto-detects device type and selects appropriate flashing protocol
  • NordicDFUFlasher: Implements Nordic DFU protocol with 1200 baud touch bootloader entry for nRF52 devices (RAK4631, T-Echo)
  • ESPToolFlasher: Implements ESP32 ROM bootloader protocol with USB-JTAG-Serial reset sequence and native USB detection for ESP32-S3 devices
  • RNodeDetector: KISS protocol implementation for device detection, ROM reading, and EEPROM provisioning
  • FirmwareDownloader: Downloads firmware from GitHub releases with progress tracking
  • FirmwarePackage: Parses firmware ZIP files and extracts embedded firmware hashes
  • SLIP and KISS codecs for serial communication framing

UI Implementation (5-step wizard)

  1. Device Selection - Lists USB devices and requests permissions
  2. Device Detection - Auto-detects board type and firmware version
  3. Firmware Selection - Choose board, frequency band, and version with download capability
  4. Flash Progress - Determinate progress bar with cancel confirmation
  5. Complete - Success/failure states with next actions

Integration

  • MainActivity: Added USB device auto-navigation with bootloader flash mode flag
  • UsbDeviceActionScreen: User-friendly screen allowing choice between flashing or configuring
  • Navigation routes integrated with existing app structure

Test Coverage

Good unit test coverage for critical components:

  • CRC16, SLIP, and KISS codec implementations
  • FirmwarePackage parsing
  • Native USB device detection logic
  • RNodeFlasher state transitions

Architecture Quality

The implementation demonstrates solid software engineering:

  • Clear separation of concerns between protocols (DFU vs ESPTool)
  • Proper state management with StateFlow for UI observation
  • Comprehensive error handling with recoverable error states
  • Platform-specific handling (ESP32 vs ESP32-S3 bootloader offsets, native USB reset sequences)
  • Device provisioning with EEPROM writes and firmware hash validation

Issues Identified

  • FirmwareDownloader lacks rate limiting and retry logic for GitHub API requests
  • FlasherViewModel has tight coupling to MainActivity via direct companion object access, breaking MVVM architecture
  • Consider extracting bootloaderFlashModeActive to a shared repository/state manager

Confidence Score: 4/5

  • This PR is safe to merge with minor improvements recommended for network reliability and architecture
  • Score reflects comprehensive implementation with good test coverage, proper error handling, and attention to hardware-specific details. The core flashing logic is well-tested and follows protocol specifications. Minor architecture improvements recommended for network resilience and MVVM separation.
  • Pay attention to FirmwareDownloader.kt (add retry logic) and FlasherViewModel.kt (decouple from MainActivity)

Important Files Changed

Filename Overview
reticulum/src/main/java/com/lxmf/messenger/reticulum/flasher/RNodeFlasher.kt Main orchestration class implementing firmware flashing with auto-detection, platform-specific flashers, and provisioning. Well-structured with proper error handling and state management.
reticulum/src/main/java/com/lxmf/messenger/reticulum/flasher/ESPToolFlasher.kt Implements ESP32 ROM bootloader protocol with USB-JTAG-Serial reset sequence, native USB detection, and multi-region flashing. Comprehensive implementation with proper timing delays.
reticulum/src/main/java/com/lxmf/messenger/reticulum/flasher/NordicDFUFlasher.kt Nordic DFU protocol implementation for nRF52 devices using 1200 baud touch bootloader entry. Clean implementation following protocol spec.
reticulum/src/main/java/com/lxmf/messenger/reticulum/flasher/FirmwareDownloader.kt Downloads firmware from GitHub releases. Missing rate limiting and retry logic for API requests.
app/src/main/java/com/lxmf/messenger/viewmodel/FlasherViewModel.kt Manages 5-step wizard UI state with firmware selection and download. Contains tight coupling to MainActivity via direct companion object access.
app/src/main/java/com/lxmf/messenger/MainActivity.kt Added USB device auto-navigation, bootloader flash mode flag, and navigation routes for flasher and USB action screen. Well-integrated with existing navigation.
reticulum/src/main/java/com/lxmf/messenger/reticulum/flasher/RNodeDetector.kt KISS protocol implementation for device detection, ROM reading, and EEPROM provisioning. Properly handles device communication with timeouts.

Sequence Diagram

sequenceDiagram
    participant User
    participant FlasherUI as RNodeFlasherScreen
    participant VM as FlasherViewModel
    participant Flasher as RNodeFlasher
    participant Detector as RNodeDetector
    participant DFU as NordicDFUFlasher
    participant ESP as ESPToolFlasher
    participant Downloader as FirmwareDownloader
    participant USB as KotlinUSBBridge
    participant Device as USB Device

    User->>FlasherUI: Open Flasher
    FlasherUI->>VM: Initialize
    VM->>Flasher: getConnectedDevices()
    Flasher->>USB: getConnectedUsbDevices()
    USB-->>Flasher: List of USB devices
    Flasher-->>VM: Device list
    VM-->>FlasherUI: Update state with devices
    
    User->>FlasherUI: Select device
    FlasherUI->>VM: selectDevice()
    VM->>Flasher: requestPermission()
    Flasher->>USB: requestPermission()
    USB-->>User: Permission dialog
    User-->>USB: Grant permission
    USB-->>Flasher: Permission granted
    Flasher-->>VM: Permission callback
    
    User->>FlasherUI: Click Continue
    FlasherUI->>VM: detectDevice()
    VM->>Flasher: detectDevice(deviceId)
    Flasher->>USB: connect(deviceId)
    USB->>Device: Connect
    Device-->>USB: Connected
    Flasher->>Detector: getDeviceInfo()
    Detector->>USB: Send KISS detect command
    USB->>Device: CMD_DETECT
    Device-->>USB: DETECT_RESP
    Detector->>USB: Read platform, MCU, board, ROM
    USB->>Device: CMD_PLATFORM, CMD_MCU, etc.
    Device-->>USB: Device info
    Detector-->>Flasher: RNodeDeviceInfo
    Flasher->>USB: disconnect()
    Flasher-->>VM: Device info
    VM-->>FlasherUI: Update with detected board
    
    User->>FlasherUI: Select firmware
    FlasherUI->>VM: selectFirmware() or downloadFirmware()
    alt Download firmware
        VM->>Flasher: firmwareDownloader
        Flasher->>Downloader: getAvailableReleases()
        Downloader->>Downloader: HTTP GET GitHub API
        Downloader-->>Flasher: Release list
        VM->>Downloader: downloadFirmware()
        Downloader->>Downloader: HTTP GET firmware ZIP
        Downloader-->>VM: Firmware data
        VM->>Flasher: firmwareRepository.saveFirmware()
    end
    
    User->>FlasherUI: Click Flash
    FlasherUI->>VM: startFlashing()
    VM->>Flasher: flashFirmware()
    Flasher->>Flasher: Verify integrity
    
    alt ESP32 device
        Flasher->>ESP: flash()
        ESP->>USB: connect(deviceId)
        ESP->>USB: enableRawMode()
        ESP->>ESP: trySyncQuick() - check if already in bootloader
        alt Not in bootloader
            ESP->>ESP: enterBootloader() - DTR/RTS sequence
            ESP->>USB: setRts/setDtr control signals
            USB->>Device: Reset to bootloader
        end
        ESP->>USB: Send ESP_SYNC command
        Device-->>USB: Sync response
        ESP->>USB: Send ESP_CHANGE_BAUDRATE
        ESP->>USB: Send ESP_SPI_ATTACH
        ESP->>USB: Send ESP_FLASH_BEGIN
        loop For each flash region
            ESP->>USB: Send ESP_FLASH_DATA packets
            USB->>Device: Write firmware blocks
            Device-->>USB: ACK
            ESP-->>VM: Progress update
            VM-->>FlasherUI: Update progress bar
        end
        ESP->>USB: Send ESP_FLASH_END
        ESP->>ESP: hardReset()
        ESP->>USB: disconnect()
    else nRF52 device
        Flasher->>DFU: flash()
        DFU->>USB: connect(deviceId, 1200 baud)
        DFU->>USB: disconnect() - triggers bootloader
        DFU->>DFU: Wait for bootloader
        DFU->>USB: connect(deviceId, 115200 baud)
        DFU->>USB: Send DFU_START_PACKET
        DFU->>USB: Send DFU_INIT_PACKET
        loop For each firmware block
            DFU->>USB: Send DFU_DATA_PACKET
            USB->>Device: Write firmware
            DFU-->>VM: Progress update
            VM-->>FlasherUI: Update progress bar
        end
        DFU->>USB: Send DFU_STOP_DATA_PACKET
        DFU->>USB: disconnect()
    end
    
    Flasher->>Flasher: Wait for device reboot
    Flasher->>Flasher: provisionDevice()
    Flasher->>USB: connect(deviceId) - with retries
    Flasher->>Detector: getDeviceInfo()
    alt Already provisioned
        Detector->>Detector: setFirmwareHash()
    else Not provisioned
        Detector->>Detector: provisionAndSetFirmwareHash()
        Detector->>USB: Write EEPROM (product, model, serial)
        Detector->>USB: Set firmware hash
        Detector->>USB: Send CMD_RESET
    end
    Flasher->>USB: disconnect()
    Flasher-->>VM: FlashState.Complete
    VM-->>FlasherUI: Show success
    FlasherUI-->>User: Flash complete!
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +46 to +71
* Get available firmware releases from GitHub.
*/
suspend fun getAvailableReleases(): List<GitHubRelease>? =
withContext(Dispatchers.IO) {
try {
val url = URL(GITHUB_RELEASES)
val connection = url.openConnection() as HttpURLConnection

connection.requestMethod = "GET"
connection.setRequestProperty("Accept", "application/vnd.github.v3+json")
connection.setRequestProperty("User-Agent", USER_AGENT)
connection.connectTimeout = CONNECT_TIMEOUT_MS
connection.readTimeout = READ_TIMEOUT_MS

if (connection.responseCode != HttpURLConnection.HTTP_OK) {
Log.e(TAG, "GitHub API returned ${connection.responseCode}")
return@withContext null
}

val responseBody = connection.inputStream.bufferedReader().readText()
json.decodeFromString<List<GitHubRelease>>(responseBody)
} catch (e: Exception) {
Log.e(TAG, "Failed to fetch releases", e)
null
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no rate limiting or retry logic for GitHub API requests - may hit rate limits or fail on transient errors

Suggested change
* Get available firmware releases from GitHub.
*/
suspend fun getAvailableReleases(): List<GitHubRelease>? =
withContext(Dispatchers.IO) {
try {
val url = URL(GITHUB_RELEASES)
val connection = url.openConnection() as HttpURLConnection
connection.requestMethod = "GET"
connection.setRequestProperty("Accept", "application/vnd.github.v3+json")
connection.setRequestProperty("User-Agent", USER_AGENT)
connection.connectTimeout = CONNECT_TIMEOUT_MS
connection.readTimeout = READ_TIMEOUT_MS
if (connection.responseCode != HttpURLConnection.HTTP_OK) {
Log.e(TAG, "GitHub API returned ${connection.responseCode}")
return@withContext null
}
val responseBody = connection.inputStream.bufferedReader().readText()
json.decodeFromString<List<GitHubRelease>>(responseBody)
} catch (e: Exception) {
Log.e(TAG, "Failed to fetch releases", e)
null
}
}
suspend fun getAvailableReleases(maxRetries: Int = 3): List<GitHubRelease>? =
withContext(Dispatchers.IO) {
repeat(maxRetries) { attempt ->
try {
val url = URL(GITHUB_RELEASES)
val connection = url.openConnection() as HttpURLConnection
connection.requestMethod = "GET"
connection.setRequestProperty("Accept", "application/vnd.github.v3+json")
connection.setRequestProperty("User-Agent", USER_AGENT)
connection.connectTimeout = CONNECT_TIMEOUT_MS
connection.readTimeout = READ_TIMEOUT_MS
if (connection.responseCode == HttpURLConnection.HTTP_FORBIDDEN) {
// Check for rate limit headers
val rateLimitRemaining = connection.getHeaderField("X-RateLimit-Remaining")?.toIntOrNull()
if (rateLimitRemaining == 0) {
Log.e(TAG, "GitHub API rate limit exceeded")
return@withContext null
}
}
if (connection.responseCode != HttpURLConnection.HTTP_OK) {
if (attempt < maxRetries - 1) {
delay(1000L * (attempt + 1)) // Exponential backoff
return@repeat
}
Log.e(TAG, "GitHub API returned ${connection.responseCode}")
return@withContext null
}
val responseBody = connection.inputStream.bufferedReader().readText()
return@withContext json.decodeFromString<List<GitHubRelease>>(responseBody)
} catch (e: Exception) {
if (attempt < maxRetries - 1) {
Log.w(TAG, "Retry attempt ${attempt + 1} failed, retrying...", e)
delay(1000L * (attempt + 1))
} else {
Log.e(TAG, "Failed to fetch releases after $maxRetries attempts", e)
return@withContext null
}
}
}
null
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: reticulum/src/main/java/com/lxmf/messenger/reticulum/flasher/FirmwareDownloader.kt
Line: 46:71

Comment:
no rate limiting or retry logic for GitHub API requests - may hit rate limits or fail on transient errors

```suggestion
    suspend fun getAvailableReleases(maxRetries: Int = 3): List<GitHubRelease>? =
        withContext(Dispatchers.IO) {
            repeat(maxRetries) { attempt ->
                try {
                    val url = URL(GITHUB_RELEASES)
                    val connection = url.openConnection() as HttpURLConnection

                    connection.requestMethod = "GET"
                    connection.setRequestProperty("Accept", "application/vnd.github.v3+json")
                    connection.setRequestProperty("User-Agent", USER_AGENT)
                    connection.connectTimeout = CONNECT_TIMEOUT_MS
                    connection.readTimeout = READ_TIMEOUT_MS

                    if (connection.responseCode == HttpURLConnection.HTTP_FORBIDDEN) {
                        // Check for rate limit headers
                        val rateLimitRemaining = connection.getHeaderField("X-RateLimit-Remaining")?.toIntOrNull()
                        if (rateLimitRemaining == 0) {
                            Log.e(TAG, "GitHub API rate limit exceeded")
                            return@withContext null
                        }
                    }

                    if (connection.responseCode != HttpURLConnection.HTTP_OK) {
                        if (attempt < maxRetries - 1) {
                            delay(1000L * (attempt + 1)) // Exponential backoff
                            return@repeat
                        }
                        Log.e(TAG, "GitHub API returned ${connection.responseCode}")
                        return@withContext null
                    }

                    val responseBody = connection.inputStream.bufferedReader().readText()
                    return@withContext json.decodeFromString<List<GitHubRelease>>(responseBody)
                } catch (e: Exception) {
                    if (attempt < maxRetries - 1) {
                        Log.w(TAG, "Retry attempt ${attempt + 1} failed, retrying...", e)
                        delay(1000L * (attempt + 1))
                    } else {
                        Log.e(TAG, "Failed to fetch releases after $maxRetries attempts", e)
                        return@withContext null
                    }
                }
            }
            null
        }
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +144 to +146
com.lxmf.messenger.MainActivity.bootloaderFlashModeActive = true
_state.update { it.copy(useManualBoardSelection = true) }
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

direct access to MainActivity companion object creates tight coupling and breaks MVVM architecture - FlasherViewModel should not directly manipulate MainActivity state

Better to use a repository or state manager:

// In a shared repository/manager
object FlasherStateManager {
    @Volatile
    var bootloaderFlashModeActive: Boolean = false
}

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: app/src/main/java/com/lxmf/messenger/viewmodel/FlasherViewModel.kt
Line: 144:146

Comment:
direct access to MainActivity companion object creates tight coupling and breaks MVVM architecture - FlasherViewModel should not directly manipulate MainActivity state

Better to use a repository or state manager:
```
// In a shared repository/manager
object FlasherStateManager {
    @Volatile
    var bootloaderFlashModeActive: Boolean = false
}
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature: rnode flasher

2 participants