Skip to content

Conversation

@haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Nov 28, 2025

  • tested working on Android 8 and 15 (permissions, connect, disconnect, read, write)

issues remaining

  • capacitor is verbose in debug logging which might impact communication handling
  • save and reboot does not fire reinitializeConnection (MSP_REBOOT)
  • connect sometimes does have deviceId but not the required UUID
  • when multiple devices available there is no dialog for selection

Summary by CodeRabbit

  • New Features

    • Android: Full Bluetooth LE support — discover, select, connect, exchange data, and receive notifications; app now declares BLE capability and requests appropriate Bluetooth permissions.
    • Bluetooth offered as a connection option alongside serial.
  • Chores / Improvements

    • Capacitor-based BLE integration with native event/permission flows, service/characteristic resolution, write/notification handling, and connection lifecycle.
    • Improved connection locking and a more robust EEPROM save flow with timeout fallback and arming-safety handling.

✏️ Tip: You can customize this high-level summary in your review settings.

@haslinghuis haslinghuis added this to the 2026.6 milestone Nov 28, 2025
@haslinghuis haslinghuis self-assigned this Nov 28, 2025
@haslinghuis haslinghuis moved this to App in 2026.6.0 Nov 28, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 28, 2025

Walkthrough

Adds end-to-end Android BLE support: manifest permissions/features, a new Capacitor Android plugin and its registration, a JS CapacitorBluetooth protocol adapter integrated into protocol selection, plus related serial backend and MSP EEPROM timing/cleanup adjustments.

Changes

Cohort / File(s) Summary
Android Manifest
android/app/src/main/AndroidManifest.xml
Adds Bluetooth permissions (BLUETOOTH, BLUETOOTH_ADMIN, ACCESS_COARSE_LOCATION with maxSdkVersion="30"; BLUETOOTH_SCAN with minSdkVersion="31" and usesPermissionFlags="neverForLocation"; BLUETOOTH_CONNECT with minSdkVersion="31") and uses-feature android.hardware.bluetooth_le required="true".
Native plugin + registration
android/app/src/main/java/betaflight/configurator/MainActivity.java, android/app/src/main/java/.../protocols/bluetooth/BetaflightBluetoothPlugin.java
Adds and registers BetaflightBluetoothPlugin; new plugin implements BLE scanning, permission handling, device discovery, connect/disconnect with GATT discovery, queued writes, notification management, event emission, timeouts, and exposes Capacitor PluginMethods (checkStatus, requestPermissions, getDevices, requestDevice, stopScan, connect, disconnect, write, startNotifications, stopNotifications).
JS BLE adapter
src/js/protocols/CapacitorBluetooth.js
New CapacitorBluetooth class (EventTarget) implementing device discovery/selection, connection lifecycle, service/characteristic resolution & caching, base64 write queueing, notification processing, and public API (getDevices, connect, disconnect, send).
Protocol integration & compatibility
src/js/serial.js, src/js/utils/checkCompatibility.js
Imports/registers CapacitorBluetooth on Android ("bluetooth" protocol path) and enables Android Bluetooth detection in checkBluetoothSupport.
Serial backend concurrency guard
src/js/serial_backend.js
Wraps non-virtual serial.connect with GUI.connect_lock set before call and cleared in a finally block to prevent concurrent connect attempts.
MSP EEPROM flow & exports
src/js/msp/MSPHelper.js
Reworks EEPROM save flow to quiet background activity, introduces sendEeprom with a finish helper and 3s fallback timer, arming-safety handling (disable arming when needed), increases initial delay to 200ms, and exposes mspHelper/exports (default and named).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant UI as Configurator UI
    participant JS as CapacitorBluetooth (JS)
    participant Bridge as Capacitor Bridge
    participant Plugin as BetaflightBluetoothPlugin (Android)
    participant BLE as Android BLE Stack

    UI->>JS: requestDevice()
    JS->>Bridge: requestDevice()
    Bridge->>Plugin: startScan(filters)
    BLE-->>Plugin: onScanResult(device)
    Plugin-->>Bridge: emit deviceDiscovered
    Bridge-->>JS: deviceDiscovered
    JS->>UI: show devices

    UI->>JS: connect(deviceId)
    JS->>Bridge: connect(deviceId)
    Bridge->>Plugin: connect -> connectGatt()
    BLE-->>Plugin: onConnectionStateChange(CONNECTED)
    Plugin->>BLE: discoverServices()
    BLE-->>Plugin: onServicesDiscovered
    Plugin-->>Bridge: emit services, connectionState
    Bridge-->>JS: services, connectionState
    JS->>UI: connection ready

    UI->>JS: send(data)
    JS->>Bridge: write(service,char,payload)
    Bridge->>Plugin: write()
    Plugin->>BLE: writeCharacteristic()
    BLE-->>Plugin: onCharacteristicWrite
    Plugin-->>Bridge: write result

    UI->>JS: startNotifications(char)
    JS->>Bridge: startNotifications()
    Bridge->>Plugin: startNotifications()
    Plugin->>BLE: enable notifications (CCCD)
    BLE-->>Plugin: onCharacteristicChanged(data)
    Plugin-->>Bridge: emit notification
    Bridge-->>JS: notification event
    JS->>UI: deliver data
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45–60 minutes

  • Areas needing careful review:
    • BetaflightBluetoothPlugin.java — Android permission branching (API levels), threading, GATT write queue, CCCD/descriptor handling, timeouts, cleanup.
    • src/js/protocols/CapacitorBluetooth.js — write queue correctness, service/characteristic resolution & caching, event synchronization, error/reconnect paths.
    • Integration: MainActivity.java, src/js/serial.js, src/js/utils/checkCompatibility.js, src/js/serial_backend.js.
    • src/js/msp/MSPHelper.js — EEPROM fallback timer, arming-safety sequence, and callback finalization.

Possibly related PRs

Suggested labels

Tested

Suggested reviewers

  • nerdCopter
  • blckmn
  • VitroidFPV

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description lacks critical sections required by the repository template and fails to provide adequate context for reviewers. Provide a comprehensive description covering: what problem is being solved, why these changes are needed, how they work, and link related issues using 'Fixes #' format. Follow the repository's contribution guidelines.
Docstring Coverage ⚠️ Warning Docstring coverage is 5.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Capacitor bluetooth plugin' clearly and concisely describes the main change: adding a Capacitor-based Bluetooth plugin implementation to the project.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (3)
android/app/src/main/AndroidManifest.xml (1)

78-81: Both bluetooth_le and usb.host are marked as required, limiting device compatibility.

Setting android:required="true" for both features means the app will only be installable on devices supporting both USB Host and Bluetooth LE. Consider whether android:required="false" would be more appropriate for one or both features, allowing the app to work on devices with only one capability while gracefully degrading functionality.

-    <uses-feature android:name="android.hardware.bluetooth_le" android:required="true"/>
+    <uses-feature android:name="android.hardware.bluetooth_le" android:required="false"/>

     <!-- USB Host feature -->
-    <uses-feature android:name="android.hardware.usb.host" android:required="true" />
+    <uses-feature android:name="android.hardware.usb.host" android:required="false" />
src/js/protocols/CapacitorBluetooth.js (1)

36-60: Base64 encoding/decoding implementation is correct for byte data.

The static analysis suggests String.fromCodePoint() and codePointAt(), but for base64 encoding of binary data (byte values 0-255), fromCharCode()/charCodeAt() are equivalent and commonly used. This is a false positive since BLE data is always byte-based.

The for-of loop suggestion is a minor style preference. Consider using for...of if you want to align with project conventions:

 const toBase64 = (buffer) => {
     if (typeof Buffer !== "undefined") {
         return Buffer.from(buffer).toString("base64");
     }
     let binary = "";
-    for (let idx = 0; idx < buffer.length; idx += 1) {
-        binary += String.fromCharCode(buffer[idx]);
+    for (const byte of buffer) {
+        binary += String.fromCharCode(byte);
     }
     return btoa(binary);
 };
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1)

919-970: Consider adding lifecycle guards to GATT callback.

While handleOnDestroy() calls cleanupGatt(), GATT callbacks can potentially be invoked after plugin destruction. Consider adding defensive checks at the start of each callback method to verify the plugin is still in a valid state.

Example pattern:

 @Override
 public void onConnectionStateChange(BluetoothGatt gatt, int status, int newState) {
+    if (bluetoothAdapter == null) {
+        return; // Plugin has been destroyed
+    }
     if (newState == BluetoothProfile.STATE_CONNECTED && status == BluetoothGatt.GATT_SUCCESS) {
         // existing implementation

Apply similar guards to onServicesDiscovered and onCharacteristicChanged.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7020cc2 and 7aae568.

📒 Files selected for processing (6)
  • android/app/src/main/AndroidManifest.xml (1 hunks)
  • android/app/src/main/java/betaflight/configurator/MainActivity.java (1 hunks)
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1 hunks)
  • src/js/protocols/CapacitorBluetooth.js (1 hunks)
  • src/js/serial.js (2 hunks)
  • src/js/utils/checkCompatibility.js (1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.

Applied to files:

  • src/js/serial.js
  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • android/app/src/main/java/betaflight/configurator/MainActivity.java
📚 Learning: 2025-11-22T21:18:08.814Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.

Applied to files:

  • src/js/serial.js
  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/MainActivity.java
📚 Learning: 2025-11-24T15:07:25.227Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.

Applied to files:

  • src/js/serial.js
  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-05-14T21:51:09.253Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

Applied to files:

  • src/js/serial.js
  • src/js/utils/checkCompatibility.js
  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/MainActivity.java
📚 Learning: 2025-10-25T21:16:32.474Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src/js/protocols/TauriSerial.js:203-259
Timestamp: 2025-10-25T21:16:32.474Z
Learning: In TauriSerial (src/js/protocols/TauriSerial.js), the requestPermissionDevice() method is not needed and not invoked. Tauri automatically discovers serial devices through the constructor's loadDevices() and startDeviceMonitoring() calls, bypassing the browser permission model that WebSerial requires. Devices are auto-detected via a 1-second polling interval without user permission prompts.

Applied to files:

  • src/js/serial.js
📚 Learning: 2025-06-09T00:33:22.959Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:33:22.959Z
Learning: In the betaflight-configurator codebase, port paths use counter suffixes for serial and bluetooth ports (e.g., "serial1", "serial2", "bluetooth1", "bluetooth2") instead of using connectionId, which means simple string matching against protocol map keys won't work for protocol selection.

Applied to files:

  • src/js/serial.js
📚 Learning: 2025-05-14T21:39:16.388Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

Applied to files:

  • src/js/utils/checkCompatibility.js
🧬 Code graph analysis (1)
src/js/protocols/CapacitorBluetooth.js (1)
src/js/protocols/devices.js (2)
  • bluetoothDevices (1-51)
  • bluetoothDevices (1-51)
🪛 GitHub Check: SonarCloud Code Analysis
src/js/protocols/CapacitorBluetooth.js

[warning] 178-178: Remove this useless assignment to variable "uuids".

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXM&open=AZrMevH6HNZ6jDqvdUXM&pullRequest=4714


[warning] 57-57: Prefer String#codePointAt() over String#charCodeAt().

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXJ&open=AZrMevH6HNZ6jDqvdUXJ&pullRequest=4714


[warning] 42-42: Prefer String.fromCodePoint() over String.fromCharCode().

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXI&open=AZrMevH6HNZ6jDqvdUXI&pullRequest=4714


[warning] 192-196: Remove this commented out code.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXN&open=AZrMevH6HNZ6jDqvdUXN&pullRequest=4714


[warning] 41-43: Expected a for-of loop instead of a for loop with this simple iteration.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXH&open=AZrMevH6HNZ6jDqvdUXH&pullRequest=4714


[failure] 87-87: Refactor this asynchronous operation outside of the constructor.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXK&open=AZrMevH6HNZ6jDqvdUXK&pullRequest=4714


[warning] 178-178: Remove the declaration of the unused 'uuids' variable.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXL&open=AZrMevH6HNZ6jDqvdUXL&pullRequest=4714

🔇 Additional comments (19)
src/js/utils/checkCompatibility.js (1)

141-151: LGTM! Android Bluetooth support correctly enabled.

This change aligns with the retrieved learnings that Android Chrome browsers support Bluetooth API, and the new CapacitorBluetooth protocol will handle the native implementation.

android/app/src/main/java/betaflight/configurator/MainActivity.java (1)

5-14: LGTM! Bluetooth plugin registration follows established pattern.

The plugin registration is correctly placed before super.onCreate() and follows the same pattern as the existing BetaflightSerialPlugin.

src/js/serial.js (1)

23-34: LGTM! Android protocols correctly include Bluetooth.

The CapacitorBluetooth protocol is properly added alongside CapacitorSerial for Android. Based on learnings, these protocol instances are singletons maintained for the application lifetime, which is the intended pattern.

Note: The Android path doesn't include the tcp (Websocket) protocol that's available on non-Android platforms. Verify this is intentional.

src/js/protocols/CapacitorBluetooth.js (5)

62-101: Constructor initialization is appropriate.

The static analysis warning about "asynchronous operation" at line 87 is a false positive. Promise.resolve() is synchronous initialization of a resolved promise for the write queue pattern. The attachNativeListeners() call is also synchronous - it registers event listeners without awaiting. Based on learnings, this fire-and-forget pattern for listener setup in protocol constructors is the established pattern in this codebase.


399-422: Write queue error handling is correct but could be clearer.

The pattern of using .catch(() => {}) on line 410 to prevent the queue from breaking while handling errors in the outer try/catch is valid. The queue continues processing even if individual writes fail, and errors are properly logged and returned.


631-642: Notification handler correctly filters by device ID.

The filtering logic on lines 635-637 ensures notifications from other devices are ignored. This is important when multiple devices might be paired. The null checks are appropriate.


571-588: LGTM! Service discovery and characteristic resolution logic is well-structured.

The dynamic service discovery pattern properly caches services, resolves pending promises, and updates the device description for the connected device. The merge logic correctly combines static profiles with discovered characteristics.


361-367: Based on my investigation, I've encountered limitations accessing the repository directly. However, I have found critical information about the API being used.

Web search results confirm that the @capacitor-community/bluetooth-le plugin (the standard Capacitor Bluetooth LE API) uses a consistent string-based parameter format for all operations:

await BleClient.disconnect(device.deviceId);  // deviceId is a STRING

This is documented in the official plugin README and npm package docs.

Given this API documentation, the review comment's concern about passing deviceId as a string to disconnect() appears to align with the correct API specification. However, I cannot definitively verify whether connect() and write() actually use an object format { deviceId: ... } as claimed, since I cannot access the repository code directly.


Verify the parameter formats in connect() and write() method calls at lines 243 and 401 to confirm whether they truly use object syntax or also pass string parameters consistently with the standard plugin API. Based on standard Capacitor Bluetooth LE documentation, all methods should accept plain string deviceId parameters.

android/app/src/main/AndroidManifest.xml (1)

67-76: ACCESS_FINE_LOCATION is required for backward compatibility with Android 11 (API ≤30); there is no conflict with neverForLocation.

The neverForLocation flag is valid for Android 12+ (API 31+) to assert that your app never derives physical location from BLE scan results. However, ACCESS_FINE_LOCATION is still necessary for older devices running Android 11 or below, where it is required to perform Bluetooth scanning regardless of the neverForLocation flag. These permissions coexist appropriately for backward compatibility. If your app targets only Android 12 and above, you could remove ACCESS_FINE_LOCATION; otherwise, keep it. Do not use ACCESS_COARSE_LOCATION as a substitute—official Android documentation specifies ACCESS_FINE_LOCATION for BLE scanning on older API levels.

android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (10)

50-73: LGTM! Plugin configuration is correct.

The permission declarations properly handle both legacy (BLUETOOTH, BLUETOOTH_ADMIN, ACCESS_FINE_LOCATION) and modern Android 12+ permissions (BLUETOOTH_SCAN, BLUETOOTH_CONNECT). The timeout constants and CCCD UUID are appropriate for BLE operations.


110-172: LGTM! Lifecycle and basic methods are well-implemented.

Proper initialization in load(), cleanup in handleOnDestroy(), and permission handling follows Capacitor best practices.


253-338: LGTM! Excellent write implementation with robust fallbacks.

The method includes comprehensive validation, smart characteristic lookup with service fallback (lines 287-296), intelligent write type selection based on supported properties (lines 319-327), and thorough error handling. The fallback logic at lines 290-295 is particularly well-designed for cases where the service UUID might not match exactly.


340-441: LGTM! Notification handling is well-designed.

The queue mechanism for pending notifications until services are discovered (line 347) is a good pattern. The characteristic lookup includes the same robust fallback logic as the write method, and cleanup is handled properly.


443-526: LGTM! Scanning implementation is correct.

The scan flow includes proper filter building, timeout handling, and cleanup. The use of SCAN_MODE_LOW_LATENCY is appropriate for user-initiated device discovery.


528-636: LGTM! Connection management is robust.

Proper API level handling for GATT connection (lines 536-540), comprehensive cleanup with exception guards (lines 617-623), and correct state management throughout.


750-762: LGTM! Correct API level handling.

Properly handles the API change in Android 13 (Tiramisu) where writeCharacteristic signature changed.


726-748: LGTM! Notification enabling is correctly implemented.

Properly distinguishes between notifications and indications (lines 735-737) and correctly writes the CCCD descriptor.


854-869: LGTM! Hex decoding is correctly implemented.

Proper validation of input length and hex digits with clear error messages.


972-1021: LGTM! Scan filtering logic is well-implemented.

The matches() method correctly handles name, prefix, and service UUID filtering with appropriate case-insensitive comparisons and null checks.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (2)

213-253: Consider explicitly rejecting a pending connect on manual disconnect

If disconnect() is called while a connection is still in progress (state CONNECTING and pendingConnectCall non‑null), the plugin relies on the Bluetooth stack to deliver a subsequent STATE_DISCONNECTED callback so that failConnect() will reject the pending call. If, for any reason, no callback arrives after cleanupGatt() (e.g., stack quirks), the JS connect() promise could remain unresolved.

A small defensive improvement is to explicitly reject any outstanding pendingConnectCall inside disconnectInternal(false) after cleanupGatt():

 private void disconnectInternal(boolean fromDestroy) {
   ConnectionState current = connectionState.getAndSet(fromDestroy ? ConnectionState.DISCONNECTED : ConnectionState.DISCONNECTING);
   if (current == ConnectionState.DISCONNECTED && !fromDestroy) {
     cleanupGatt();
     return;
   }

   if (connectTimeoutRunnable != null) {
     mainHandler.removeCallbacks(connectTimeoutRunnable);
     connectTimeoutRunnable = null;
   }

   String lastDeviceId = connectedDeviceId;
   cleanupGatt();
   activeNotifications.clear();
+
+  // Defensively fail any in‑flight connect() so JS does not hang if no callback arrives
+  if (!fromDestroy && pendingConnectCall != null) {
+    PluginCall call = pendingConnectCall;
+    pendingConnectCall = null;
+    call.setKeepAlive(false);
+    call.reject("Disconnected");
+  }
+
   connectionState.set(ConnectionState.DISCONNECTED);
   notifyConnectionState(false, "disconnected", lastDeviceId);
   rejectPendingStartNotifications("Device disconnected");
 }

445-529: Scanning helpers are well‑encapsulated; minor concurrency improvement possible

startLeScan, handleScanResult, and stopScanInternal form a clean unit: they refresh the scanner, build filters, wire callbacks, push discoveries into discoveredDevices, emit deviceDiscovered events, and ensure timeouts and callbacks are cleaned up in all paths.

One minor concurrency refinement: pendingScanCall is written on the plugin thread and read from the scan callback thread and timeout runnable. Declaring it volatile would make this intent explicit and avoid any subtle visibility races:

-  private PluginCall pendingScanCall;
+  private volatile PluginCall pendingScanCall;

Similarly, you might consider volatile for pendingConnectCall since it’s read/written across the plugin, timeout, and GATT callback threads.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7aae568 and 2bccc88.

📒 Files selected for processing (1)
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
🔇 Additional comments (15)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (15)

56-111: Solid plugin annotation and concurrency‑safe core state

Use of a single permission alias, AtomicReference<ConnectionState>, ConcurrentHashMap for device/notification maps, and ConcurrentLinkedQueue for pending notification calls is appropriate here and fixes the earlier race on pendingStartNotificationCalls. No issues in this block.


112-131: Lifecycle handling looks correct

load() initializes the adapter/scanner once, and handleOnDestroy() performs a best‑effort scan stop and disconnect before delegating to super.handleOnDestroy(). This is a clean shutdown pattern for the plugin.


132-175: Permission / status / device list API design is coherent

checkStatus, requestPermissions/onPermissionResult, and getDevices expose a clear, minimal surface and correctly gate behavior on the Capacitor permission alias. Returning deviceId, available, enabled, and connected gives JS everything it needs without leaking extra state. No changes needed.


176-210: Request/stop scan flow is well structured

requestDevice enforces single active scan via pendingScanCall, merges required/optional services into a unified filter, uses keep‑alive correctly, and stopScan simply delegates to the internal stop helper. Behavior here matches expectations for a Web‑Bluetooth‑style requestDevice.


255-341: Write path is robust and MSP‑friendly

Argument validation, UUID parsing with a service fallback search, property checks for write vs write‑without‑response, flexible payload decoding (base64/hex/utf8), and API‑level‑aware writeCharacteristic handling are all well done. Resolving bytesSent from the payload length is straightforward and sufficient given you’re not tracking the async write result. No issues spotted here.


342-443: Notification start/stop flow and queuing behavior look correct

startNotifications correctly queues calls until servicesDiscovered is true, then startNotificationsInternal performs validation, characteristic lookup with service fallback, enables notifications, and tracks active subscriptions. stopNotifications handles both key‑based and characteristic‑based removal and gracefully returns success when nothing is active. The queuing via pendingStartNotificationCalls and clearing on reconnect/disconnect is clean.


530-577: GATT open and failure handling are thorough

openGatt correctly chooses the transport overload based on API level and catches SecurityException to surface a clear error when permissions are missing. failConnect centralizes state reset, GATT cleanup, timeout teardown, connect‑call rejection, connection state event emission, and pending notification rejection, which keeps the error paths consistent. This section looks solid.


578-629: Connection lifecycle and cleanup logic are coherent

connectedSuccessfully cancels the timeout, resolves the pending connect call once, and emits a connectionState event. disconnectInternal drives the state machine through DISCONNECTINGDISCONNECTED, cancels timeouts, clears notifications, and sends a final connection event, while cleanupGatt defensively disconnects/closes and resets fields. Overall, the lifecycle management is clear and consistent with the rest of the plugin.


640-685: Readiness checks and device resolution are appropriate

ensureBluetoothReady and ensureConnected provide clear, early rejections with actionable error messages, and resolveDevice first consults the discovery cache before falling back to getRemoteDevice with error logging for invalid IDs. This is a good separation of concerns.


686-707: Device payload construction is comprehensive

createDevicePayload includes ID, name, bond state, RSSI, and advertised service UUIDs, which matches what the JS side typically needs for device selection UIs. Conditional handling of missing ScanRecord/UUIDs is safe. No issues here.


709-727: Scan rejection and main‑thread utility function are clean

rejectPendingScan safely logs, rejects the pending call (if any), and then delegates to stopScanInternal for cleanup, avoiding duplicate logic. runOnMainThread correctly short‑circuits when already on the main looper. This block looks good.


728-765: Notification enable/disable and write submission are implemented correctly

enableNotifications uses setCharacteristicNotification plus the standard 0x2902 (client config) descriptor with indicate vs notify selection, and submitWrite cleanly switches between the Android 13+ writeCharacteristic overload and the legacy setValue+writeCharacteristic call. Exceptions for permission issues are logged and surfaced as false. No further changes needed.


766-903: Helper utilities are robust and defensive

decodePayload, hexToBytes, parseUuidArray, parseUuid, notificationKey, buildScanFilters, and safeDeviceId are all straightforward, validate inputs, and handle malformed data gracefully (e.g., hex length/characters, UUID parsing errors). Using Collections.emptyList() defaults keeps null‑handling simple. This section is in good shape.


919-977: Gatt callback logic and compatibility handling are solid

onConnectionStateChange advances the connection state, kicks off service discovery on successful connect, and funnels all disconnects through failConnect() to unify error and teardown handling. onServicesDiscovered marks servicesDiscovered, flushes queued notifications, logs the layout, and emits a structured services payload. The addition of the deprecated two‑argument onCharacteristicChanged that forwards to the three‑argument Android 13+ overload resolves the earlier compatibility gap for Android 12 and below. Notification events include device, service, characteristic, and base64 payload as expected.


979-1028: ScanCriteria matching logic is clear and flexible

The ScanCriteria helper cleanly encodes acceptAll, exact name, prefix (case‑insensitive), and advertised service UUID constraints, short‑circuiting early where possible. Treating acceptAll as an override that bypasses other filters is a reasonable design choice as long as it matches the JS expectations. No functional problems found.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1)

961-975: Null safety issue confirmed—add guard against null value in onCharacteristicChanged

The Android Bluetooth API documentation confirms that BluetoothGattCharacteristic.getValue() can return null due to GATT edge cases, uninitialized cache, or stack-specific behavior (especially on pre-API 33). When the deprecated 2-arg onCharacteristicChanged forwards characteristic.getValue() to the 3-arg override, a null value reaches Base64.encodeToString(value, ...), causing a NullPointerException and dropping the notification silently.

Add a null-safety guard to handle this gracefully:

@Override
public void onCharacteristicChanged(BluetoothGatt gatt,
                                    BluetoothGattCharacteristic characteristic,
                                    byte[] value) {
+    if (value == null || characteristic == null || characteristic.getService() == null) {
+        Log.w(TAG, "Notification with null value or characteristic, ignoring");
+        return;
+    }
     JSObject payload = new JSObject();
     payload.put("deviceId", connectedDeviceId);
     payload.put("service", characteristic.getService().getUuid().toString());
     payload.put("characteristic", characteristic.getUuid().toString());
     payload.put("value", Base64.encodeToString(value, Base64.NO_WRAP));
     notifyListeners("notification", payload);
}

This prevents crashes on documented Bluetooth stack quirks while keeping normal notifications unchanged.

🧹 Nitpick comments (3)
android/app/src/main/AndroidManifest.xml (1)

58-79: Confirm that hard‑requiring BLE is intended

The Bluetooth permission set looks aligned with the new plugin, but declaring android.hardware.bluetooth_le with android:required="true" means the app won’t be installable on devices without BLE, even if USB would still work. If you want “USB‑only but no BLE” devices to remain supported, consider android:required="false" here.

android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (2)

176-202: Scan filters still restrict results when acceptAllDevices is true

requestDevice merges services and optionalServices into combinedFilter, and startLeScan always feeds criteria.serviceUuids into buildScanFilters, so Android still filters by service UUID even when acceptAllDevices is true. This diverges from the WebBluetooth / PWA pattern where { acceptAllDevices: true, optionalServices: [...] } shows all devices and does not filter by services at scan time.

Since ScanCriteria.matches() already short‑circuits on acceptAll, a minimal way to align behavior is to skip platform scan filters when acceptAll is true:

-        List<ScanFilter> filters = buildScanFilters(criteria.serviceUuids);
+        // When acceptAll is true, don't restrict by service UUIDs at scan time.
+        List<ScanFilter> filters = buildScanFilters(
+            criteria.acceptAll ? Collections.emptyList() : criteria.serviceUuids
+        );

This preserves service‑based filtering when explicitly requested (acceptAllDevices == false && services.length > 0), while letting the Capacitor path behave like WebBluetooth.js when using acceptAllDevices + optionalServices.

Based on learnings, this would align the CapacitorBluetooth behavior with the existing WebBluetooth.js flow.

Also applies to: 445-484, 904-913, 979-1027


557-576: Disconnect handling can emit duplicate/ambiguous connectionState events

disconnectInternal(false) already calls cleanupGatt(), sets connectionState to DISCONNECTED, and sends a "disconnected" connectionState event with the last non‑null deviceId. Later, onConnectionStateChange(... STATE_DISCONNECTED ...) also calls cleanupGatt() and failConnect(...), which in turn sends another connectionState event, but now with deviceId coming from connectedDeviceId after it has been nulled by cleanupGatt().

Result: for an explicit disconnect you can see two events (one “disconnected” with a deviceId, one “Connection failed: Disconnected” with null deviceId), and for remote disconnects the event may carry a null deviceId.

Consider distinguishing “expected disconnect” from “connect failure/remote drop”, and capturing the deviceId before calling cleanupGatt() in the callback path so listeners always get a consistent deviceId for the last connection. For example, gate the failConnect(...) call on connectionState != DISCONNECTING, or split out a dedicated “remoteDisconnect” helper that snapshots the deviceId before cleanup.

Also applies to: 596-614, 919-931

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4d85241 and 4af5c2e.

📒 Files selected for processing (7)
  • android/app/src/main/AndroidManifest.xml (1 hunks)
  • android/app/src/main/java/betaflight/configurator/MainActivity.java (1 hunks)
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1 hunks)
  • src/js/protocols/CapacitorBluetooth.js (1 hunks)
  • src/js/serial.js (2 hunks)
  • src/js/serial_backend.js (2 hunks)
  • src/js/utils/checkCompatibility.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • src/js/serial_backend.js
  • src/js/serial.js
  • src/js/protocols/CapacitorBluetooth.js
  • src/js/utils/checkCompatibility.js
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/MainActivity.java
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-28T22:51:11.691Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/MainActivity.java
  • android/app/src/main/AndroidManifest.xml
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-05-14T21:51:09.253Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/MainActivity.java
📚 Learning: 2025-11-22T21:18:08.814Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/MainActivity.java
📚 Learning: 2025-09-19T20:41:44.286Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-19T20:42:20.332Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-09T20:02:33.475Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4597
File: src/js/utils/common.js:95-127
Timestamp: 2025-09-09T20:02:33.475Z
Learning: In the Betaflight Configurator codebase, the Chromium v140 compatibility issue with sortSelect was resolved by rewriting the implementation to use native DOM APIs (Array.from, select.options, select.remove, select.add) instead of jQuery DOM manipulation methods (this.children, this.empty().append). The fix still performs DOM operations but avoids the specific jQuery methods that were causing issues in Chromium v140.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-11T19:10:56.992Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4567
File: locales/en/messages.json:7724-7728
Timestamp: 2025-08-11T19:10:56.992Z
Learning: In the Betaflight Configurator project, only English language entries are added directly to locales/en/messages.json. Translations for other languages are handled through an outsourced process and should not be suggested as part of code reviews.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-10T18:26:10.136Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-19T17:55:51.714Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4708
File: src/js/tabs/firmware_flasher.js:0-0
Timestamp: 2025-11-19T17:55:51.714Z
Learning: In the betaflight-configurator project, only current Chrome and Edge-based browsers are officially supported, and users are expected to keep their browsers up-to-date for security reasons. This means ES2023 features like Array.prototype.toSorted() and Object.groupBy() can be safely used without polyfills or transpilation.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
🔇 Additional comments (3)
android/app/src/main/java/betaflight/configurator/MainActivity.java (1)

5-13: Bluetooth plugin registration is correct and consistent

Registering BetaflightBluetoothPlugin alongside BetaflightSerialPlugin before super.onCreate follows the existing pattern and should integrate cleanly with Capacitor’s bridge lifecycle. No changes needed here.

android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (2)

94-111: Overall plugin structure and thread‑safety look solid

Using AtomicReference for connectionState, ConcurrentHashMap/ConcurrentLinkedQueue for shared collections, and a main‑thread Handler for GATT entry points gives a good concurrency baseline. Error handling in write, notification setup/teardown, and UUID/payload parsing are defensive and clear. Aside from the specific points noted in other comments, the class is well‑structured and fits the existing Capacitor plugin patterns.

Also applies to: 167-175, 283-340, 373-443, 771-774, 871-886


204-210: Based on my research into Capacitor plugin architecture, I can now assess the review comment.

The web search confirms that Capacitor uses a setKeepAlive/saveCall pattern for async operations. When a PluginCall is marked as keep-alive and saved, it must be explicitly resolve()d or reject()d to settle the JavaScript promise. If neither occurs, the promise will hang indefinitely.

The review comment correctly identifies this architectural concern: if requestDevice() keeps the call alive and saves it, but stopScan() only calls stopScanInternal() without settling the promise, the JavaScript promise will indeed remain unresolved.

However, since the repository clone failed, I cannot verify:

  1. The actual implementation of stopScan() and stopScanInternal()
  2. Whether pendingScanCall and rejectPendingScan() actually exist
  3. How requestDevice() manages the call lifecycle
  4. What occurs at lines 515-528 ("Also applies to")

The concern raised is architecturally valid and follows documented Capacitor best practices. The suggested fix (routing through rejectPendingScan()) is the correct pattern for settling an outstanding async call.


stopScan may leave the original requestDevice call unresolved if the promise settlement pattern is not followed

In Capacitor's async callback model, when a PluginCall is kept alive (via setKeepAlive(true)), it must be explicitly resolved or rejected to settle the JavaScript promise. If requestDevice() saves the call and stopScan() calls stopScanInternal() without settling it, the JS promise will hang indefinitely.

Route user-initiated stops through rejectPendingScan(...) to ensure the promise is settled:

@PluginMethod
public void stopScan(PluginCall call) {
-    stopScanInternal();
+    if (pendingScanCall != null) {
+        // Cancel the outstanding requestDevice call as well.
+        rejectPendingScan("Scan stopped");
+    } else {
+        stopScanInternal();
+    }
     JSObject result = new JSObject();
     result.put("stopped", true);
     call.resolve(result);
}

This keeps internal error/timeout paths unchanged while making stopScan() behavior predictable for callers.

@haslinghuis
Copy link
Member Author

Copy link
Member

@nerdCopter nerdCopter left a comment

Choose a reason for hiding this comment

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

  • 619ba7f APK -- i can only partially test because i've never been able to connect/bluetooth my APEXF7 flightcontroller; however it does "see" it.
  • Android 15
Image

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/js/protocols/CapacitorBluetooth.js (1)

178-178: Remove unused variable.

The uuids variable is computed but never used. Based on past review discussions, the acceptAllDevices: true workaround (line 189) is intentional due to UUID mismatch issues on Android, so the uuids computation is no longer needed.

Apply this diff:

 async requestPermissionDevice() {
     let newPermissionDevice = null;
-    const uuids = bluetoothDevices.map((device) => device.serviceUuid).filter(Boolean);

     try {
         await BetaflightBluetooth.requestPermissions(); // prompt once

Based on learnings, acceptAllDevices pattern is used because Android devices advertise different service UUIDs than what's in the bluetoothDevices catalog.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4af5c2e and 15a29f0.

📒 Files selected for processing (7)
  • android/app/src/main/AndroidManifest.xml (1 hunks)
  • android/app/src/main/java/betaflight/configurator/MainActivity.java (1 hunks)
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1 hunks)
  • src/js/protocols/CapacitorBluetooth.js (1 hunks)
  • src/js/serial.js (2 hunks)
  • src/js/serial_backend.js (2 hunks)
  • src/js/utils/checkCompatibility.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • android/app/src/main/java/betaflight/configurator/MainActivity.java
  • src/js/serial_backend.js
  • src/js/utils/checkCompatibility.js
  • src/js/serial.js
🧰 Additional context used
🧠 Learnings (21)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.760Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
📚 Learning: 2025-11-30T18:10:26.760Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.760Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • android/app/src/main/AndroidManifest.xml
  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-28T22:51:11.691Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • android/app/src/main/AndroidManifest.xml
  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-28T22:41:59.374Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-05-14T21:51:09.253Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • android/app/src/main/AndroidManifest.xml
  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-09-19T20:41:44.286Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-09-19T20:42:20.332Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-09-09T20:02:33.475Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4597
File: src/js/utils/common.js:95-127
Timestamp: 2025-09-09T20:02:33.475Z
Learning: In the Betaflight Configurator codebase, the Chromium v140 compatibility issue with sortSelect was resolved by rewriting the implementation to use native DOM APIs (Array.from, select.options, select.remove, select.add) instead of jQuery DOM manipulation methods (this.children, this.empty().append). The fix still performs DOM operations but avoids the specific jQuery methods that were causing issues in Chromium v140.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-11T19:10:56.992Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4567
File: locales/en/messages.json:7724-7728
Timestamp: 2025-08-11T19:10:56.992Z
Learning: In the Betaflight Configurator project, only English language entries are added directly to locales/en/messages.json. Translations for other languages are handled through an outsourced process and should not be suggested as part of code reviews.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-10T18:26:10.136Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-19T17:55:51.714Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4708
File: src/js/tabs/firmware_flasher.js:0-0
Timestamp: 2025-11-19T17:55:51.714Z
Learning: In the betaflight-configurator project, only current Chrome and Edge-based browsers are officially supported, and users are expected to keep their browsers up-to-date for security reasons. This means ES2023 features like Array.prototype.toSorted() and Object.groupBy() can be safely used without polyfills or transpilation.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-05-14T21:39:16.388Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

Applied to files:

  • android/app/src/main/AndroidManifest.xml
  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-24T15:07:25.227Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-09T00:33:22.959Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:33:22.959Z
Learning: In the betaflight-configurator codebase, port paths use counter suffixes for serial and bluetooth ports (e.g., "serial1", "serial2", "bluetooth1", "bluetooth2") instead of using connectionId, which means simple string matching against protocol map keys won't work for protocol selection.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-19T22:13:09.136Z
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4521
File: src/js/protocols/WebSerial.js:148-151
Timestamp: 2025-06-19T22:13:09.136Z
Learning: In WebSerial.js, there's a timing issue where the cached `this.ports` array doesn't immediately reflect newly permitted devices after `requestPermissionDevice()` completes. The `getDevices()` method needs to refresh the device list from the browser API to return accurate data immediately following a permission request and user acceptance.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-10-25T21:16:32.474Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src/js/protocols/TauriSerial.js:203-259
Timestamp: 2025-10-25T21:16:32.474Z
Learning: In TauriSerial (src/js/protocols/TauriSerial.js), the requestPermissionDevice() method is not needed and not invoked. Tauri automatically discovers serial devices through the constructor's loadDevices() and startDeviceMonitoring() calls, bypassing the browser permission model that WebSerial requires. Devices are auto-detected via a 1-second polling interval without user permission prompts.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-22T21:18:08.814Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-06T17:26:50.201Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4687
File: src/js/protocols/TauriSerial.js:299-310
Timestamp: 2025-11-06T17:26:50.201Z
Learning: In tauri-plugin-serialplugin Android development, the Android project must be initialized via `tauri android init` before the USB support patches (AndroidManifest.xml permissions, device_filter.xml, MainActivity.kt) can be applied. Without initialization, the plugin returns empty results from available_ports_direct() because the generated Android project structure and USB permissions don't exist yet.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
🧬 Code graph analysis (1)
src/js/protocols/CapacitorBluetooth.js (2)
src/js/protocols/devices.js (2)
  • bluetoothDevices (1-51)
  • bluetoothDevices (1-51)
src/js/gui_log.js (1)
  • gui_log (7-21)
🪛 GitHub Check: SonarCloud Code Analysis
src/js/protocols/CapacitorBluetooth.js

[warning] 178-178: Remove this useless assignment to variable "uuids".

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXM&open=AZrMevH6HNZ6jDqvdUXM&pullRequest=4714


[warning] 41-43: Expected a for-of loop instead of a for loop with this simple iteration.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXH&open=AZrMevH6HNZ6jDqvdUXH&pullRequest=4714


[warning] 57-57: Prefer String#codePointAt() over String#charCodeAt().

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXJ&open=AZrMevH6HNZ6jDqvdUXJ&pullRequest=4714


[failure] 87-87: Refactor this asynchronous operation outside of the constructor.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXK&open=AZrMevH6HNZ6jDqvdUXK&pullRequest=4714


[warning] 42-42: Prefer String.fromCodePoint() over String.fromCharCode().

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXI&open=AZrMevH6HNZ6jDqvdUXI&pullRequest=4714


[warning] 178-178: Remove the declaration of the unused 'uuids' variable.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXL&open=AZrMevH6HNZ6jDqvdUXL&pullRequest=4714

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: deploy
🔇 Additional comments (12)
src/js/protocols/CapacitorBluetooth.js (3)

62-101: LGTM: Constructor follows established patterns.

The constructor properly initializes state, attaches native listeners, and follows the singleton pattern established in the codebase. The fire-and-forget listener attachment is appropriate since protocol implementations live for the entire application lifetime.

Based on learnings, protocol implementations are instantiated once and never destroyed, so cleanup methods for event listeners are not needed.


204-276: LGTM: Connection flow with robust service resolution.

The connection flow properly:

  • Manages connection state flags
  • Waits for device characteristics with timeout-based fallback
  • Validates complete service/characteristic availability before proceeding
  • Starts notifications after successful connection
  • Handles errors and cleanup appropriately

The waitForDeviceCharacteristics pattern with pending promises and timeouts provides robust coordination with the native service discovery events.


375-414: LGTM: Write operation with proper queuing and error handling.

The write queue pattern using promise chaining ensures sequential writes while avoiding race conditions. The base64 encoding, payload conversion, and error handling are all appropriate for BLE transport.

android/app/src/main/AndroidManifest.xml (2)

59-77: LGTM: Version-aware Bluetooth permissions properly configured.

The permission declarations correctly implement version-specific Bluetooth permission handling:

  • Legacy permissions (BLUETOOTH, BLUETOOTH_ADMIN, ACCESS_COARSE_LOCATION) are scoped to Android 8-11 with maxSdkVersion="30"
  • New permissions (BLUETOOTH_SCAN, BLUETOOTH_CONNECT) are scoped to Android 12+ with minSdkVersion="31"
  • The neverForLocation flag on BLUETOOTH_SCAN is a good privacy practice

Based on learnings, this structure is necessary because Android 8-11 requires legacy permissions while Android 12+ requires the new runtime permissions, and the Java code must conditionally request permissions using SDK version checks.


68-69: Verify if ACCESS_FINE_LOCATION is necessary for BLE scanning.

ACCESS_FINE_LOCATION is declared but may not be required. According to Android permission requirements for BLE, ACCESS_COARSE_LOCATION is typically sufficient for BLE scanning on Android 6-11. Review the Bluetooth implementation code to confirm whether fine location precision is actually needed for your use case, or if coarse location (with maxSdkVersion="30") would be adequate.

android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (7)

159-242: LGTM: Version-aware permission handling implemented correctly.

The permission request implementation properly handles Android version differences:

  • Uses getRequiredPermissions() to determine version-specific permissions
  • Android 12+ requests BLUETOOTH_SCAN and BLUETOOTH_CONNECT
  • Android 8-11 requests legacy BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION
  • Uses native ActivityCompat.requestPermissions with proper callback handling
  • Stores pending call for later resolution

This matches the learning about version-aware permission handling and avoids requesting non-existent permissions on older Android versions.


115-116: LGTM: Thread-safe collection for pending notification calls.

The switch from ArrayList to ConcurrentLinkedQueue addresses the race condition identified in previous reviews. The volatile servicesDiscovered flag combined with the thread-safe queue ensures safe access from multiple threads.


536-604: LGTM: Robust BLE scanning with timeout and criteria matching.

The scanning implementation properly:

  • Builds scan filters from service UUIDs
  • Uses low-latency scan mode for responsive discovery
  • Implements scan timeout with cleanup
  • Matches devices against name/prefix/service criteria
  • Emits device discovery events
  • Resolves pending scan call on first match
  • Handles scan failures gracefully

The batch result handling and criteria matching logic are well-designed.


622-646: LGTM: GATT connection with proper API level handling.

The openGatt method correctly:

  • Uses TRANSPORT_LE parameter on Android 6+ (API 23+)
  • Falls back to two-parameter signature on older versions
  • Handles SecurityException for missing permissions
  • Sets up connection timeout with cleanup
  • Uses main thread for GATT operations

843-855: LGTM: Write operation with API level compatibility.

The write submission handles Android API differences correctly:

  • Android 13+ (Tiramisu): Uses new three-parameter writeCharacteristic API
  • Earlier versions: Uses deprecated two-parameter API with setValue/setWriteType
  • Properly handles SecurityException

1024-1050: LGTM: Service discovery with comprehensive event emission.

The onServicesDiscovered callback:

  • Sets servicesDiscovered flag to unblock pending notification calls
  • Flushes queued startNotifications calls
  • Builds complete service/characteristic hierarchy with properties
  • Emits detailed services event for JavaScript layer

This provides the JavaScript side with all information needed for dynamic service resolution.


918-943: LGTM: Pending notification call queue with proper lifecycle.

The queue management correctly:

  • Queues startNotifications calls when services aren't discovered yet
  • Flushes the queue once services are available
  • Rejects pending calls on disconnect or connection failure
  • Uses thread-safe ConcurrentLinkedQueue

This ensures notifications can be started immediately after connection without race conditions.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/js/protocols/CapacitorBluetooth.js (1)

36-59: Optional: simplify base64 helpers with modern iteration APIs

toBase64 / fromBase64 manually build strings with an indexed for loop and String.fromCharCode / charCodeAt. Functionally fine, but you could switch to for...of on the Uint8Array and codePointAt() for slightly clearer, more idiomatic code if you touch this again. Given the Buffer paths handle most environments, this is purely a readability/style tweak, not a bug.

android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1)

61-76: Align permission checks with the new runtime permission flow and clean up unused callback

You’ve moved to explicit runtime permission handling via getRequiredPermissions() + ActivityCompat.requestPermissions, but:

  • The plugin annotation still defines a bluetooth alias with strings = {}, and
  • ensureBluetoothReady() still checks getPermissionState("bluetooth"), while
  • @PermissionCallback onPermissionResult is no longer used (you don’t call requestPermissionForAlias anymore).

This means the Capacitor alias state is effectively decoupled from the real OS permission state, and the early “Bluetooth permission not granted” guard may never trigger even if Android permissions are denied.

Consider simplifying and making these consistent by:

  1. Dropping the alias-based permission path (or leaving the alias empty but not relying on it), and
  2. Basing readiness purely on your version-aware permission helper:
-    private boolean ensureBluetoothReady(PluginCall call) {
-        if (getPermissionState("bluetooth") != PermissionState.GRANTED) {
-            call.reject("Bluetooth permission not granted");
-            return false;
-        }
+    private boolean ensureBluetoothReady(PluginCall call) {
+        // Check OS permissions using the same logic as requestPermissions()
+        String[] requiredPermissions = getRequiredPermissions();
+        for (String permission : requiredPermissions) {
+            if (ContextCompat.checkSelfPermission(getContext(), permission)
+                    != PackageManager.PERMISSION_GRANTED) {
+                call.reject("Bluetooth permission not granted");
+                return false;
+            }
+        }
         if (bluetoothAdapter == null) {
             call.reject("Bluetooth adapter unavailable");
             return false;
         }
@@
         return true;
     }

And, if you keep this approach, you can remove the now-unused onPermissionResult @PermissionCallback method and the commented-out requestPermissions implementation to avoid confusion.

This will make the permission story clearer and ensure ensureBluetoothReady() reflects the actual runtime permission state on both Android 8–11 and 12+.

Also applies to: 159-210, 244-254, 731-745

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 15a29f0 and 00e1ac4.

📒 Files selected for processing (2)
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1 hunks)
  • src/js/protocols/CapacitorBluetooth.js (1 hunks)
🧰 Additional context used
🧠 Learnings (21)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.760Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
📚 Learning: 2025-11-28T22:51:11.691Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-28T22:41:59.374Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-24T15:07:25.227Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-22T21:18:08.814Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-09T00:33:22.959Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:33:22.959Z
Learning: In the betaflight-configurator codebase, port paths use counter suffixes for serial and bluetooth ports (e.g., "serial1", "serial2", "bluetooth1", "bluetooth2") instead of using connectionId, which means simple string matching against protocol map keys won't work for protocol selection.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-19T22:13:09.136Z
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4521
File: src/js/protocols/WebSerial.js:148-151
Timestamp: 2025-06-19T22:13:09.136Z
Learning: In WebSerial.js, there's a timing issue where the cached `this.ports` array doesn't immediately reflect newly permitted devices after `requestPermissionDevice()` completes. The `getDevices()` method needs to refresh the device list from the browser API to return accurate data immediately following a permission request and user acceptance.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-10-25T21:16:32.474Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src/js/protocols/TauriSerial.js:203-259
Timestamp: 2025-10-25T21:16:32.474Z
Learning: In TauriSerial (src/js/protocols/TauriSerial.js), the requestPermissionDevice() method is not needed and not invoked. Tauri automatically discovers serial devices through the constructor's loadDevices() and startDeviceMonitoring() calls, bypassing the browser permission model that WebSerial requires. Devices are auto-detected via a 1-second polling interval without user permission prompts.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-09-19T20:41:44.286Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-19T20:42:20.332Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-05-14T21:51:09.253Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-05-14T21:39:16.388Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-11-30T18:10:26.760Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.760Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-06T17:26:50.201Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4687
File: src/js/protocols/TauriSerial.js:299-310
Timestamp: 2025-11-06T17:26:50.201Z
Learning: In tauri-plugin-serialplugin Android development, the Android project must be initialized via `tauri android init` before the USB support patches (AndroidManifest.xml permissions, device_filter.xml, MainActivity.kt) can be applied. Without initialization, the plugin returns empty results from available_ports_direct() because the generated Android project structure and USB permissions don't exist yet.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-09-09T20:02:33.475Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4597
File: src/js/utils/common.js:95-127
Timestamp: 2025-09-09T20:02:33.475Z
Learning: In the Betaflight Configurator codebase, the Chromium v140 compatibility issue with sortSelect was resolved by rewriting the implementation to use native DOM APIs (Array.from, select.options, select.remove, select.add) instead of jQuery DOM manipulation methods (this.children, this.empty().append). The fix still performs DOM operations but avoids the specific jQuery methods that were causing issues in Chromium v140.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-11T19:10:56.992Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4567
File: locales/en/messages.json:7724-7728
Timestamp: 2025-08-11T19:10:56.992Z
Learning: In the Betaflight Configurator project, only English language entries are added directly to locales/en/messages.json. Translations for other languages are handled through an outsourced process and should not be suggested as part of code reviews.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-10T18:26:10.136Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-19T17:55:51.714Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4708
File: src/js/tabs/firmware_flasher.js:0-0
Timestamp: 2025-11-19T17:55:51.714Z
Learning: In the betaflight-configurator project, only current Chrome and Edge-based browsers are officially supported, and users are expected to keep their browsers up-to-date for security reasons. This means ES2023 features like Array.prototype.toSorted() and Object.groupBy() can be safely used without polyfills or transpilation.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
🧬 Code graph analysis (1)
src/js/protocols/CapacitorBluetooth.js (2)
src/js/gui_log.js (1)
  • gui_log (7-21)
src/js/protocols/devices.js (2)
  • bluetoothDevices (1-51)
  • bluetoothDevices (1-51)
🪛 GitHub Actions: Deployment (PR and Push)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java

[error] 1055-1055: Compilation failed: cannot find symbol 'value' referenced in 'if (characteristic == null || value == null) {'. Likely 'value' is undefined or out of scope in this context (BetaflightBluetoothPlugin.java:1055).

🪛 GitHub Check: SonarCloud Code Analysis
src/js/protocols/CapacitorBluetooth.js

[failure] 87-87: Refactor this asynchronous operation outside of the constructor.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXK&open=AZrMevH6HNZ6jDqvdUXK&pullRequest=4714


[warning] 42-42: Prefer String.fromCodePoint() over String.fromCharCode().

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXI&open=AZrMevH6HNZ6jDqvdUXI&pullRequest=4714


[warning] 41-43: Expected a for-of loop instead of a for loop with this simple iteration.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXH&open=AZrMevH6HNZ6jDqvdUXH&pullRequest=4714


[warning] 57-57: Prefer String#codePointAt() over String#charCodeAt().

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXJ&open=AZrMevH6HNZ6jDqvdUXJ&pullRequest=4714

🔇 Additional comments (1)
src/js/protocols/CapacitorBluetooth.js (1)

62-647: Capacitor Bluetooth adapter design looks solid and consistent with existing protocols

Connection lifecycle, dynamic service/characteristic resolution, CRC workaround, serialized write queue, and native listener wiring all look coherent and in line with the existing Serial/WebBluetooth patterns. No blocking issues from this side.

@haslinghuis
Copy link
Member Author

@haslinghuis
Copy link
Member Author

@coderabbitai - on any tab when using save and reboot it does not fire reinitializeConnection (MSP_REBOOT) in serial_backend.js - it does not even reach the function

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 1, 2025

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get the reinitializeConnection function from serial_backend.js
echo "=== reinitializeConnection function in serial_backend.js ==="
ast-grep --pattern $'function reinitializeConnection($$$) {
  $$$
}'

# Also search for reinitializeConnection declarations
rg -nP 'function\s+reinitializeConnection|const\s+reinitializeConnection|reinitializeConnection\s*=' src/js/serial_backend.js -A 30

# Search for MSP_REBOOT usage and how it triggers reinitializeConnection
echo -e "\n=== MSP_REBOOT usage ==="
rg -nP 'MSP_REBOOT' src/js/ -C 3

# Look for where reinitializeConnection is called
echo -e "\n=== reinitializeConnection calls ==="
rg -nP 'reinitializeConnection\s*\(' src/js/ -C 3

Length of output: 6146


🏁 Script executed:

#!/bin/bash
# Examine CapacitorBluetooth.js disconnect handling
echo "=== CapacitorBluetooth disconnect method ==="
cat src/js/protocols/CapacitorBluetooth.js | sed -n '/disconnect[[:space:]]*(/,/^[[:space:]]*}[[:space:]]*$/p' | head -50

# Look for event listeners in CapacitorBluetooth
echo -e "\n=== Event listeners in CapacitorBluetooth ==="
rg -nP 'addListener|addEventListener|removeListener' src/js/protocols/CapacitorBluetooth.js -C 2

# Check for connection state handling
echo -e "\n=== Connection state handling in CapacitorBluetooth ==="
rg -nP 'connectionState|onDisconnect|closeRequested' src/js/protocols/CapacitorBluetooth.js -C 3

Length of output: 2537


🏁 Script executed:

#!/bin/bash
# Look at WebBluetooth.js to compare disconnect/reconnect handling
echo "=== WebBluetooth disconnect handling for comparison ==="
rg -nP 'disconnect|gattserverdisconnected|onDisconnect' src/js/protocols/WebBluetooth.js -C 5 | head -80

# Check for any 'close' event handling
echo -e "\n=== Close event handling ==="
rg -nP "dispatchEvent.*close|'close'|\"close\"" src/js/protocols/ -C 2

Length of output: 3125


🏁 Script executed:

#!/bin/bash
# Check for CustomEvent dispatching in WebBluetooth and CapacitorBluetooth
echo "=== CustomEvent dispatches in WebBluetooth ==="
rg -nP "dispatchEvent.*new CustomEvent" src/js/protocols/WebBluetooth.js -C 2

echo -e "\n=== CustomEvent dispatches in CapacitorBluetooth ==="
rg -nP "dispatchEvent.*new CustomEvent" src/js/protocols/CapacitorBluetooth.js -C 2

# Check what serial_backend.js does with "connect" events
echo -e "\n=== Connect event handling in serial_backend ==="
rg -nP "'connect'|\"connect\"|addEventListener.*connect" src/js/serial_backend.js -C 5

# Look at the handleDisconnect methods more closely
echo -e "\n=== handleDisconnect in WebBluetooth ==="
ast-grep --pattern $'handleDisconnect() {
  $$$
}'

Length of output: 5808


🏁 Script executed:

#!/bin/bash
# Check handleRemoteDisconnect in CapacitorBluetooth
echo "=== handleRemoteDisconnect in CapacitorBluetooth ==="
rg -nP 'handleRemoteDisconnect' src/js/protocols/CapacitorBluetooth.js -B 2 -A 30

# Check if there's a pattern of how disconnect events should propagate
echo -e "\n=== Disconnect event propagation patterns ==="
rg -nP 'addEventListener.*disconnect|removeEventListener.*disconnect' src/js/serial_backend.js -C 5

Length of output: 7362


🏁 Script executed:

#!/bin/bash
# Look at what the serial object connection listener does in serial_backend.js
echo "=== Serial connection event listener setup ==="
rg -nP 'serial\.addEventListener|connection\.addEventListener' src/js/serial_backend.js -B 5 -A 10

# Check for disconnect handling in serial_backend
echo -e "\n=== Disconnect handling in serial_backend ==="
rg -nP 'function.*disconnect|const.*disconnect.*=|disconnect.*function' src/js/serial_backend.js -C 5

Length of output: 2699


🏁 Script executed:

#!/bin/bash
# Examine the Android plugin's connection state change handling
echo "=== onConnectionStateChange in BetaflightBluetoothPlugin.java ==="
rg -nP 'onConnectionStateChange' android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java -B 3 -A 20

# Check how connectionState events are notified
echo -e "\n=== connectionState event notifications ==="
rg -nP 'notifyListeners.*connectionState|connectionState.*notifyListeners' android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java -B 5 -A 5

# Check the BluetoothGattCallback implementation
echo -e "\n=== BluetoothGattCallback implementation ==="
rg -nP 'BluetoothGattCallback|new BluetoothGattCallback' android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java -B 2 -A 30

Length of output: 4587


🏁 Script executed:

#!/bin/bash
# Check if there's any special handling for GATT connection state
echo "=== GATT connection state handling ==="
rg -nP 'STATE_DISCONNECTED|STATE_CONNECTED|newState' android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java -B 2 -A 5

# Look for disconnect method in the plugin
echo -e "\n=== Disconnect method in plugin ==="
rg -nP '@PluginMethod.*disconnect|public void disconnect' android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java -B 2 -A 30

Length of output: 1884


🏁 Script executed:

#!/bin/bash
# Check the failConnect implementation
echo "=== failConnect method ==="
rg -nP 'private.*failConnect|void failConnect' android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java -B 2 -A 20

# Check connectedSuccessfully implementation
echo -e "\n=== connectedSuccessfully method ==="
rg -nP 'private.*connectedSuccessfully|void connectedSuccessfully' android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java -B 2 -A 20

# Check notifyConnectionState calls
echo -e "\n=== notifyConnectionState calls ==="
rg -nP 'notifyConnectionState' android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java -B 3 -A 3

Length of output: 2471


🏁 Script executed:

#!/bin/bash
# Check disconnectInternal implementation
echo "=== disconnectInternal method ==="
rg -nP 'private.*disconnectInternal|void disconnectInternal' android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java -B 2 -A 30

# Look for cleanupGatt
echo -e "\n=== cleanupGatt method ==="
rg -nP 'private.*cleanupGatt|void cleanupGatt' android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java -B 2 -A 25

Length of output: 2118


🏁 Script executed:

#!/bin/bash
# Check the connect flow and pendingConnectCall handling
echo "=== pendingConnectCall usage ==="
rg -nP 'pendingConnectCall' android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java -B 2 -A 3

# Check if there's a connected device tracking
echo -e "\n=== connectedDevice tracking ==="
rg -nP 'connectedDevice\s*=' android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java -B 2 -A 3

Length of output: 976


🏁 Script executed:

#!/bin/bash
# The user says reinitializeConnection is not being called at all
# Check GUI.tab_switch_cleanup implementation
echo "=== GUI.tab_switch_cleanup implementation ==="
rg -nP 'tab_switch_cleanup.*=|function.*tab_switch_cleanup' src/js/ -B 2 -A 30

# Check if there's anything special about how save and reboot is triggered
echo -e "\n=== Save and reboot button handlers ==="
rg -nP 'save.*reboot|reboot.*save|MSP_EEPROM_WRITE' src/js/msp/MSPHelper.js -B 5 -A 10 | head -80

Length of output: 4334


🏁 Script executed:

#!/bin/bash
# Check how save_to_eeprom is called with reboot flag
echo "=== save_to_eeprom calls with reboot ==="
rg -nP 'save_to_eeprom.*true|saveToEeprom.*true' src/js/tabs/ -B 3 -A 3 | head -100

# Check the saveToEeprom function signature
echo -e "\n=== saveToEeprom function ==="
rg -nP 'function.*saveToEeprom|const.*saveToEeprom|saveToEeprom.*function' src/js/msp/MSPHelper.js -B 2 -A 15

Length of output: 71


🏁 Script executed:

#!/bin/bash
# Check GUI object initialization and tab_switch_cleanup
echo "=== GUI object tab_switch_cleanup ==="
rg -nP 'tab_switch_cleanup' src/js/gui.js -B 5 -A 20

# Also check if there's a GUI initialization
echo -e "\n=== GUI object definition ==="
rg -nP 'export.*GUI|const GUI|GUI\s*=' src/js/gui.js -B 2 -A 10 | head -50

Length of output: 1297


🏁 Script executed:

#!/bin/bash
# Check a few tab cleanup implementations to see how they handle callbacks
echo "=== Tab cleanup method examples ==="
rg -nP 'cleanup.*function|cleanup\s*\(' src/js/tabs/pid_tuning.js -B 2 -A 15 | head -40

echo -e "\n=== Another tab cleanup example ==="
rg -nP 'cleanup.*function|cleanup\s*\(' src/js/tabs/motors.js -B 2 -A 15 | head -40

echo -e "\n=== And one more ==="
rg -nP 'cleanup.*function|cleanup\s*\(' src/js/tabs/receiver.js -B 2 -A 15 | head -40

Length of output: 2638


🏁 Script executed:

#!/bin/bash
# Check if the issue is specific to any tabs
echo "=== All tab cleanup method signatures ==="
fd -e js . src/js/tabs/ --exec rg -nP 'cleanup.*\(.*\)|cleanup:\s*function' {} \; | head -60

# See if any tabs don't properly call the callback
echo -e "\n=== Tabs that might not call callback ==="
rg -nP 'cleanup.*callback' src/js/tabs/ -A 20 | grep -v 'callback()' | head -60

Length of output: 4273


🏁 Script executed:

#!/bin/bash
# Since the user mentioned "any tab", let's check if there's a common pattern
# Let's look at what happens when a tab cleanup doesn't call the callback properly
echo "=== Looking for tabs without callback invocation ==="
for file in src/js/tabs/*.js; do
    if rg -q 'cleanup.*function|cleanup:' "$file"; then
        echo "=== $file ==="
        rg -nP 'cleanup.*function|cleanup:' "$file" -A 30 | head -40
        echo ""
    fi
done | head -200

Length of output: 5551


@haslinghuis I found the issue! The problem is that several tabs have cleanup functions that don't invoke the callback parameter. This breaks the save and reboot flow because reinitializeConnection is never called.

Here's the flow:

  1. MSPHelper.save_to_eeprom(reboot=true) calls GUI.tab_switch_cleanup(() => reinitializeConnection())
  2. GUI.tab_switch_cleanup delegates to TABS[active_tab].cleanup(callback)
  3. If that tab's cleanup doesn't call callback(), then reinitializeConnection() never fires

Tabs with missing callback invocations:

  1. src/js/tabs/power.js (lines 543-545):
power.cleanup = function (callback) {
    // Empty - callback never called!
};
  1. src/js/tabs/setup_osd.js (lines 66-68):
setup_osd.cleanup = function (callback) {
    // Empty - callback never called!
};
  1. src/js/tabs/transponder.js (lines 338-340):
transponder.cleanup = function (callback) {
    // Empty - callback never called!
};
  1. src/js/tabs/cli.js (lines 542-560) - doesn't call callback when CLI is active

Solution: All tab cleanup functions must invoke the callback:

// Add this to each problematic cleanup function
if (callback) callback();

This is not specific to the Capacitor Bluetooth implementation - it would affect save and reboot on any connection type when on these tabs.


🧠 Learnings used
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.

Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4583
File: src/js/tabs/firmware_flasher.js:949-961
Timestamp: 2025-09-02T07:45:48.606Z
Learning: In src/js/tabs/firmware_flasher.js, the config file loading code path after firmware loading (in the load_file click handler) cannot be reached when UF2 firmware is loaded, according to the maintainer blckmn. This code is maintained for backward compatibility with unified target settings and will be refactored in a future PR.

Learnt from: ctzsnooze
Repo: betaflight/betaflight-configurator PR: 4627
File: src/js/tabs/onboard_logging.js:504-506
Timestamp: 2025-09-28T12:56:15.659Z
Learning: The Betaflight Configurator MSP queue can become overwhelmed during dataflash reads when retry logic is used, even with minimal retries (MAX_SIMPLE_RETRIES = 1), because failed blocks cause exponential queue growth. The solution is to eliminate retries entirely and skip failed blocks to maintain download progress and prevent stalls.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

♻️ Duplicate comments (1)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1)

1029-1041: Add null safety check for characteristic.getService().

Line 1037 calls characteristic.getService().getUuid() without verifying that getService() returns non-null. If the service is unavailable, this will throw NPE.

Apply this diff:

 @Override
 public void onCharacteristicChanged(BluetoothGatt gatt, BluetoothGattCharacteristic characteristic, byte[] value) {
-    if (characteristic == null || value == null) {
+    if (characteristic == null || characteristic.getService() == null || value == null) {
         Log.w(TAG, "Received notification with null characteristic or value");
         return;
     }
     JSObject payload = new JSObject();
     payload.put("deviceId", connectedDeviceId);
     payload.put("service", characteristic.getService().getUuid().toString());
     payload.put("characteristic", characteristic.getUuid().toString());
     payload.put("value", Base64.encodeToString(value, Base64.NO_WRAP));
     notifyListeners("notification", payload);
 }
🧹 Nitpick comments (1)
src/js/serial_backend.js (1)

106-165: Connection lock usage is sound; consider guarding against synchronous serial.connect throws

The new GUI.connect_lock guard around serial.connect(...) correctly prevents overlapping connect attempts (including across different protocols) and the .finally ensures the lock is cleared on both resolve and reject. Because JS is single-threaded, the if (!GUI.connect_lock ...) check at Line 109 remains safe even though the lock is set slightly later.

One small defensive improvement: if serial.connect were ever to throw synchronously before returning a Promise, GUI.connect_lock would stay true and block further connects. Wrapping the call in a try { ... } finally { GUI.connect_lock = false; } (or a try/catch that resets the lock before re‑throwing) would make the guard robust to that scenario, while keeping the current behavior for normal Promise-based errors.

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 182da54 and b888cab.

📒 Files selected for processing (8)
  • android/app/src/main/AndroidManifest.xml (1 hunks)
  • android/app/src/main/java/betaflight/configurator/MainActivity.java (2 hunks)
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (1 hunks)
  • src/js/msp/MSPHelper.js (1 hunks)
  • src/js/protocols/CapacitorBluetooth.js (1 hunks)
  • src/js/serial.js (2 hunks)
  • src/js/serial_backend.js (1 hunks)
  • src/js/utils/checkCompatibility.js (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/js/serial.js
  • android/app/src/main/java/betaflight/configurator/MainActivity.java
  • src/js/utils/checkCompatibility.js
🧰 Additional context used
🧠 Learnings (30)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: In betaflight-configurator’s CapacitorBluetooth.js, native plugin listeners (notification, services, connectionState) must be attached exactly once per app lifetime. Re-attaching them during writes/connect cycles leads to duplicate handlers, increased JS workload, MSP queue pressure, and can prevent the MSP_EEPROM_WRITE callback from firing (so reinitializeConnection is not reached). Implement an idempotent _attachNativeListenersOnce() guard.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-02T17:46:47.859Z
Learning: BetaflightBluetoothPlugin.java: maintainers prefer write(PluginCall) to be “write-only” — no per-call UUID parsing or characteristic lookups. Resolve and cache the write characteristic once in onServicesDiscovered() (prefer PROPERTY_WRITE_NO_RESPONSE; else PROPERTY_WRITE). write() should read payload params from call, use the cached writeCharacteristic, pick write type, and submitWrite; clear the cache in cleanupGatt().
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:38:41.581Z
Learning: On BLE (Android) Save & Reboot, status pull intervals continue during MSP_EEPROM_WRITE, overwhelming the MSP queue and causing the EEPROM ack to time out so reinitializeConnection never runs. Solution: before EEPROM write, call GUI.interval_kill_all() and MSP.callbacks_cleanup(), and add a 3s fallback to proceed to reboot if the ack is missed.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: CapacitorBluetooth.connect should only dispatch the 'open' event after GATT services are resolved and notifications are started; announcing open earlier causes the first MSP write to fail with “Unable to write characteristic” and contributes to early queue load.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.784Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
📚 Learning: 2025-11-28T22:51:11.691Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • android/app/src/main/AndroidManifest.xml
📚 Learning: 2025-12-01T17:09:13.412Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: In betaflight-configurator’s CapacitorBluetooth.js, native plugin listeners (notification, services, connectionState) must be attached exactly once per app lifetime. Re-attaching them during writes/connect cycles leads to duplicate handlers, increased JS workload, MSP queue pressure, and can prevent the MSP_EEPROM_WRITE callback from firing (so reinitializeConnection is not reached). Implement an idempotent _attachNativeListenersOnce() guard.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • src/js/msp/MSPHelper.js
📚 Learning: 2025-11-28T22:41:59.374Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-12-01T17:09:13.412Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: CapacitorBluetooth.connect should only dispatch the 'open' event after GATT services are resolved and notifications are started; announcing open earlier causes the first MSP write to fail with “Unable to write characteristic” and contributes to early queue load.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-24T15:07:25.227Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-09T00:32:21.385Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-06-09T00:33:22.959Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:33:22.959Z
Learning: In the betaflight-configurator codebase, port paths use counter suffixes for serial and bluetooth ports (e.g., "serial1", "serial2", "bluetooth1", "bluetooth2") instead of using connectionId, which means simple string matching against protocol map keys won't work for protocol selection.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-06-19T22:13:09.136Z
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4521
File: src/js/protocols/WebSerial.js:148-151
Timestamp: 2025-06-19T22:13:09.136Z
Learning: In WebSerial.js, there's a timing issue where the cached `this.ports` array doesn't immediately reflect newly permitted devices after `requestPermissionDevice()` completes. The `getDevices()` method needs to refresh the device list from the browser API to return accurate data immediately following a permission request and user acceptance.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • src/js/serial_backend.js
📚 Learning: 2025-10-25T21:16:32.474Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src/js/protocols/TauriSerial.js:203-259
Timestamp: 2025-10-25T21:16:32.474Z
Learning: In TauriSerial (src/js/protocols/TauriSerial.js), the requestPermissionDevice() method is not needed and not invoked. Tauri automatically discovers serial devices through the constructor's loadDevices() and startDeviceMonitoring() calls, bypassing the browser permission model that WebSerial requires. Devices are auto-detected via a 1-second polling interval without user permission prompts.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • src/js/serial_backend.js
📚 Learning: 2025-12-02T17:46:47.859Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-02T17:46:47.859Z
Learning: BetaflightBluetoothPlugin.java: maintainers prefer write(PluginCall) to be “write-only” — no per-call UUID parsing or characteristic lookups. Resolve and cache the write characteristic once in onServicesDiscovered() (prefer PROPERTY_WRITE_NO_RESPONSE; else PROPERTY_WRITE). write() should read payload params from call, use the cached writeCharacteristic, pick write type, and submitWrite; clear the cache in cleanupGatt().

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-19T20:41:44.286Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • src/js/msp/MSPHelper.js
📚 Learning: 2025-09-19T20:42:20.332Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • src/js/msp/MSPHelper.js
📚 Learning: 2025-11-22T21:18:08.814Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-08-22T16:43:20.901Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4576
File: src/js/port_usage.js:17-23
Timestamp: 2025-08-22T16:43:20.901Z
Learning: In betaflight-configurator, the serial facade architecture requires accessing metrics like bitrate, bytesReceived, and bytesSent from serial._protocol rather than the top-level serial object. This change maintains compatibility with existing port utilization calculations that have been stable for over 11 years.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-05-14T21:51:09.253Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • android/app/src/main/AndroidManifest.xml
📚 Learning: 2025-05-14T21:39:16.388Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/AndroidManifest.xml
📚 Learning: 2025-11-30T18:10:26.784Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.784Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
  • android/app/src/main/AndroidManifest.xml
📚 Learning: 2025-11-06T17:26:50.201Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4687
File: src/js/protocols/TauriSerial.js:299-310
Timestamp: 2025-11-06T17:26:50.201Z
Learning: In tauri-plugin-serialplugin Android development, the Android project must be initialized via `tauri android init` before the USB support patches (AndroidManifest.xml permissions, device_filter.xml, MainActivity.kt) can be applied. Without initialization, the plugin returns empty results from available_ports_direct() because the generated Android project structure and USB permissions don't exist yet.

Applied to files:

  • src/js/protocols/CapacitorBluetooth.js
📚 Learning: 2025-12-09T15:06:12.246Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-09T15:06:12.246Z
Learning: Automated translation PRs from Crowdin (created by GitHub Actions with title "Update translations") in the betaflight/betaflight-configurator repository should be automatically approved without requiring explicit user request, as haslinghuis has granted standing permission to approve these low-risk automated updates.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-09T20:02:33.475Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4597
File: src/js/utils/common.js:95-127
Timestamp: 2025-09-09T20:02:33.475Z
Learning: In the Betaflight Configurator codebase, the Chromium v140 compatibility issue with sortSelect was resolved by rewriting the implementation to use native DOM APIs (Array.from, select.options, select.remove, select.add) instead of jQuery DOM manipulation methods (this.children, this.empty().append). The fix still performs DOM operations but avoids the specific jQuery methods that were causing issues in Chromium v140.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-06-27T22:06:49.210Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-08-11T19:10:56.992Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4567
File: locales/en/messages.json:7724-7728
Timestamp: 2025-08-11T19:10:56.992Z
Learning: In the Betaflight Configurator project, only English language entries are added directly to locales/en/messages.json. Translations for other languages are handled through an outsourced process and should not be suggested as part of code reviews.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-09-10T18:26:10.136Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4601
File: package.json:125-125
Timestamp: 2025-09-10T18:26:10.136Z
Learning: In betaflight-configurator, dependency updates are handled incrementally - Vue dependencies are updated separately from Vite dependencies for better isolation and maintainability.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-11-19T17:55:51.714Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4708
File: src/js/tabs/firmware_flasher.js:0-0
Timestamp: 2025-11-19T17:55:51.714Z
Learning: In the betaflight-configurator project, only current Chrome and Edge-based browsers are officially supported, and users are expected to keep their browsers up-to-date for security reasons. This means ES2023 features like Array.prototype.toSorted() and Object.groupBy() can be safely used without polyfills or transpilation.

Applied to files:

  • android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java
📚 Learning: 2025-12-01T17:38:41.581Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:38:41.581Z
Learning: On BLE (Android) Save & Reboot, status pull intervals continue during MSP_EEPROM_WRITE, overwhelming the MSP queue and causing the EEPROM ack to time out so reinitializeConnection never runs. Solution: before EEPROM write, call GUI.interval_kill_all() and MSP.callbacks_cleanup(), and add a 3s fallback to proceed to reboot if the ack is missed.

Applied to files:

  • src/js/msp/MSPHelper.js
📚 Learning: 2025-08-27T19:36:13.733Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/tabs/pid_tuning.js:80-82
Timestamp: 2025-08-27T19:36:13.733Z
Learning: MSP_STATUS_EX calls in serial_backend.js and setup.js are intentionally unguarded because they are used for status fetching on page load to initialize profiles and are safe to call on older firmware versions. Only conditional/optional MSP_STATUS_EX usage (like in pid_tuning.js) needs version gating to API_VERSION_25_12.

Applied to files:

  • src/js/msp/MSPHelper.js
📚 Learning: 2025-09-28T12:56:15.659Z
Learnt from: ctzsnooze
Repo: betaflight/betaflight-configurator PR: 4627
File: src/js/tabs/onboard_logging.js:504-506
Timestamp: 2025-09-28T12:56:15.659Z
Learning: The Betaflight Configurator MSP queue can become overwhelmed during dataflash reads when retry logic is used, even with minimal retries (MAX_SIMPLE_RETRIES = 1), because failed blocks cause exponential queue growth. The solution is to eliminate retries entirely and skip failed blocks to maintain download progress and prevent stalls.

Applied to files:

  • src/js/msp/MSPHelper.js
📚 Learning: 2025-09-28T16:39:04.171Z
Learnt from: ctzsnooze
Repo: betaflight/betaflight-configurator PR: 4627
File: src/js/FileSystem.js:101-107
Timestamp: 2025-09-28T16:39:04.171Z
Learning: In the Betaflight Configurator dataflash download implementation, removing await from FileSystem.writeChunk() and closeFile() is intentional to prevent MSP queue overwhelm. Awaiting file I/O operations slows the download loop enough to cause MSP timeouts and queue growth, leading to stalls. The non-blocking approach prioritizes download throughput over immediate file write error detection.

Applied to files:

  • src/js/msp/MSPHelper.js
🧬 Code graph analysis (2)
src/js/protocols/CapacitorBluetooth.js (1)
src/js/protocols/devices.js (2)
  • bluetoothDevices (1-51)
  • bluetoothDevices (1-51)
src/js/serial_backend.js (3)
src/js/gui.js (1)
  • GUI (473-473)
src/js/serial.js (2)
  • serial (232-232)
  • serial (232-232)
src/js/port_handler.js (4)
  • PortHandler (16-50)
  • selectedPort (140-140)
  • selectedPort (190-190)
  • selectedPort (279-279)
🪛 GitHub Check: SonarCloud Code Analysis
src/js/protocols/CapacitorBluetooth.js

[failure] 87-87: Refactor this asynchronous operation outside of the constructor.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXK&open=AZrMevH6HNZ6jDqvdUXK&pullRequest=4714


[warning] 41-43: Expected a for-of loop instead of a for loop with this simple iteration.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXH&open=AZrMevH6HNZ6jDqvdUXH&pullRequest=4714


[warning] 57-57: Prefer String#codePointAt() over String#charCodeAt().

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXJ&open=AZrMevH6HNZ6jDqvdUXJ&pullRequest=4714


[warning] 42-42: Prefer String.fromCodePoint() over String.fromCharCode().

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrMevH6HNZ6jDqvdUXI&open=AZrMevH6HNZ6jDqvdUXI&pullRequest=4714

src/js/msp/MSPHelper.js

[warning] 2973-2973: Unexpected negated condition.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrgvBneYqGoADY79vbQ&open=AZrgvBneYqGoADY79vbQ&pullRequest=4714


[failure] 2955-2955: Expected { after 'if' condition.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrgvBneYqGoADY79vbP&open=AZrgvBneYqGoADY79vbP&pullRequest=4714


[failure] 2947-2947: Expected { after 'if' condition.

See more on https://sonarcloud.io/project/issues?id=betaflight_betaflight-configurator&issues=AZrgvBneYqGoADY79vbO&open=AZrgvBneYqGoADY79vbO&pullRequest=4714

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: deploy
🔇 Additional comments (12)
src/js/msp/MSPHelper.js (1)

2935-2969: BLE save/reboot flow: quieting + fallback look correct and should resolve missed MSP_EEPROM_WRITE acks

The new writeConfiguration flow does the right things for the BLE issues we discussed:

  • Pre‑save GUI.interval_kill_all() and MSP.callbacks_cleanup() to stop background status pulls and stale callbacks before MSP_EEPROM_WRITE, which directly targets the queue‑starvation problem on Android BLE.
  • sendEeprom uses a finished guard plus a 3s fallback timer, so either the EEPROM ack or the timeout will call finish() exactly once, ensuring GUI.tab_switch_cleanup(() => reinitializeConnection()) and callback are always reached when reboot is true.
  • The fallback path still allows a late MSP ack to arrive safely: the timer is cleared when the ack handler runs (if it’s still pending), and the finished flag prevents duplicate reboot/callback execution.
  • Arming safety is preserved: when the FC is currently armed, you still send MSP_ARMING_DISABLE before scheduling the EEPROM write, but you no longer block on its callback, which avoids deadlock under BLE load.

This is aligned with the prior root‑cause analysis of BLE queue overload and missed EEPROM callbacks. Based on learnings, this structure is appropriate and low risk.

It would still be good to sanity‑check one thing in real usage: on “save without reboot” flows, GUI.interval_kill_all() and MSP.callbacks_cleanup() will also run. Please verify that the relevant tabs re‑establish any needed status intervals after a plain save (or that it’s acceptable for them to stay quiet until the user navigates away/back).

android/app/src/main/AndroidManifest.xml (1)

59-75: LGTM! Version-aware Bluetooth permissions correctly implemented.

The manifest properly declares Bluetooth permissions with SDK version guards to support both Android 8-11 and Android 12+, matching the required pattern. The neverForLocation flag on BLUETOOTH_SCAN and the required="true" BLE feature are appropriate.

Based on learnings, this permission structure aligns with Android's version-specific requirements.

android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java (6)

61-115: LGTM! Plugin structure correctly implements version-aware permission handling.

The empty @Permission annotation is correct since permissions are handled via native Android API with version-specific branching. The cached write characteristic fields (writeCharacteristic, writeNoResponseSupported, cachedWriteType) align with the learning that the write characteristic should be resolved once in onServicesDiscovered() and reused.

Based on learnings, this structure properly supports the version-aware permission workflow.


147-228: LGTM! Version-aware permission handling correctly implemented.

The permission flow properly branches based on Android version:

  • Android 12+ requests BLUETOOTH_SCAN and BLUETOOTH_CONNECT
  • Android 8-11 requests BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION

This matches the solution discussed in past comments and properly uses native Android permission APIs with Capacitor's handleRequestPermissionsResult() callback.

Based on learnings, this implementation resolves the Android 8 "Permissions denied" issue.


460-508: LGTM! Scan start correctly guarded against SecurityException.

The try-catch block around bluetoothLeScanner.startScan() prevents app crashes when Bluetooth permissions are missing or revoked at runtime. The scan timeout is only scheduled after successful scan start, which is the correct pattern.

Per past comments, this change was critical for preventing crashes when permissions aren't in the expected state.


332-355: LGTM! Write method correctly uses cached characteristic.

The write() method reads payload params from the call, uses the cached writeCharacteristic, picks the cached write type, and calls submitWrite without per-call UUID lookups. This matches the maintainer preference from learnings for a "write-only" implementation.

Based on learnings, the characteristic is cached once in onServicesDiscovered() and reused here.


957-1011: LGTM! Service discovery correctly caches write characteristic and delays connection success.

Lines 962-987 resolve and cache the write characteristic once, preferring WRITE_NO_RESPONSE over WRITE, which matches the maintainer preference from learnings. Line 1010 calls connectedSuccessfully() only after services are discovered, ensuring the JS 'open' event is delayed until GATT services and notifications are ready.

Based on learnings, this addresses the early "Unable to write characteristic" issue and supports the write-only write() method.


640-652: LGTM! Cached characteristic correctly reset on disconnect.

Lines 647-649 reset writeCharacteristic, writeNoResponseSupported, and cachedWriteType on all disconnect paths. This ensures the cache doesn't hold stale characteristics after disconnect or reboot.

Based on learnings, this addresses the issue where writes could occur using a stale cached characteristic after reboot.

src/js/protocols/CapacitorBluetooth.js (4)

599-649: LGTM! Native listeners correctly attached once per app lifetime.

Lines 600-604 implement an idempotent guard using the nativeListenersReady flag. The attachNativeListeners() method returns early if already called, preventing duplicate handlers that could cause MSP queue pressure and prevent MSP_EEPROM_WRITE callbacks from firing.

Based on learnings, this addresses the issue where re-attaching listeners during write cycles led to increased JS workload and prevented reinitializeConnection from being reached.


208-280: LGTM! Connection flow correctly delays 'open' event until services and notifications are ready.

Lines 243-246 wait for device characteristics (including GATT services resolution), line 255 starts notifications, and line 270 dispatches the 'connect' event only after both are complete. This ensures the first MSP write doesn't fail with "Unable to write characteristic."

Based on learnings, delaying the 'open' event until after GATT services are resolved and notifications are started prevents early queue load and write failures.


464-597: LGTM! Service resolution implements robust async handling with caching and timeout fallback.

The service resolution flow properly waits for GATT services to be discovered:

  1. Checks cached services first (lines 469-472)
  2. Creates a pending promise if characteristics incomplete (line 474)
  3. Resolves when native 'services' event arrives (lines 567-584)
  4. Falls back to provided description if timeout occurs (line 519)

This ensures the connection flow waits for service discovery before allowing writes, preventing early "Unable to write characteristic" errors.


379-418: LGTM! Send method implements correct write queue pattern.

The write queue (lines 395-406) ensures writes are serialized and the queue continues even if a write fails (line 406). The callback pattern provides backward compatibility with the existing serial API. The early checks (lines 380-389) prevent writes when disconnected or characteristic unavailable.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 Security Hotspot
4.9% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@github-actions
Copy link
Contributor

@haslinghuis
Copy link
Member Author

haslinghuis commented Dec 10, 2025

@coderabbitai checked implementation CapacitorSerial and CapacitorTcp in https://github.com/betaflight/betaflight-configurator/ to compare with CapicitorBluetooth.

Issues to resolve

  • logging is too verbose
  • read / write is too slow (should be refactored to only read or write)
  • there is no dialog when permissions are required to choose a device

Gemini Answer
That's a fantastic update on the connection plugins! I've reviewed the general approach to your CapacitorSerial and CapacitorTcp plugins and your new BLE implementation. The custom plugin approach is definitely the right choice for the level of control Betaflight requires.

Here is a breakdown of the key points for your BLE implementation and advice on optimization.

I. Review of Custom Plugins

Your implementation of CapacitorSerial and CapacitorTcp aligns with the best practices for Capacitor binary communication:

  1. Native Binary Handling: Both plugins correctly use raw streams (InputStream/OutputStream or serial port equivalents) rather than line-based/string-based methods.
  2. Base64 Encoding: Using Base64 to shuttle binary data across the Capacitor bridge is the standard, reliable method.
  3. Event-Driven Read Thread: The dedicated read thread architecture (as seen in the serial PR) is crucial for performance and handling continuous, unsolicited telemetry data from the Flight Controller (FC).

II. BLE Implementation Feedback (PR #4714)

The issues you've identified with the BLE implementation are common pain points in cross-platform mobile Bluetooth development.

  1. Excessive Logging (Too Verbose)

    Diagnosis: Verbose logging, while useful for initial debugging, can severely impact performance, especially on Android. Log buffer exhaustion, repeated I/O calls to the logcat, and CPU cycles spent formatting strings can introduce unnecessary latency.

Recommendation:

  • Implement log levels in your native code (e.g., Kotlin/Java).
  • By default, set the log level to WARNING or ERROR.
  • Only enable DEBUG or VERBOSE logging via a CLI setting or a developer option in the Configurator that sets a preference flag read by the native plugin. This keeps the production app fast but allows for deep debugging when needed.
  1. Slow Read/Write Performance (Refactoring)

The performance issues are likely due to the inherent nature of how BLE transactions are queued on the native side and the low default throughput.

Refactoring Strategy (Read/Write Separation):

  • Goal: Don't have one native method that tries to do both. A read() should only read; a write() should only write.

    • Write Optimization (The Priority): The search results confirm that Android's BLE stack queues writes and often waits for the onCharacteristicWrite callback before sending the next one.
    • Solution A (Flow Control): Implement a Write Queue in your native code. Do not call writeCharacteristic() for the next chunk until the onCharacteristicWrite() callback for the previous chunk has been received. This ensures reliability and prevents the OS from discarding writes, but it maintains the inherent serial delay.
    • Solution B (Maximum Throughput): Use BluetoothGatt.requestMtu(256) or higher immediately after connecting. Increasing the Maximum Transmission Unit (MTU) allows you to send larger data packets, dramatically reducing the number of write operations and thus minimizing the queuing delay.
    • Solution C (Best Effort): If the Betaflight FC supports it, set the Write Type to WRITE_TYPE_NO_RESPONSE for high-frequency MSP data that is not critical (like fast telemetry). This bypasses the wait for the ACK, but data loss is possible.
  • Read Optimization: Use Notifications (setCharacteristicNotification(true)). This is an asynchronous, push-based model. It eliminates polling (read()) and is far more efficient for the continuous MSP telemetry stream. Your native plugin's event loop should wait for the onCharacteristicChanged callback, which then triggers notifyListeners to the JavaScript side.

That's a fantastic update on the connection plugins! I've reviewed the general approach to your CapacitorSerial and CapacitorTcp plugins and your new BLE implementation. The custom plugin approach is definitely the right choice for the level of control Betaflight requires.

Here is a breakdown of the key points for your BLE implementation and advice on optimization.
I. Review of Custom Plugins

Your implementation of CapacitorSerial and CapacitorTcp aligns with the best practices for Capacitor binary communication:

Native Binary Handling: Both plugins correctly use raw streams (InputStream/OutputStream or serial port equivalents) rather than line-based/string-based methods.

Base64 Encoding: Using Base64 to shuttle binary data across the Capacitor bridge is the standard, reliable method.

Event-Driven Read Thread: The dedicated read thread architecture (as seen in the serial PR) is crucial for performance and handling continuous, unsolicited telemetry data from the Flight Controller (FC).

II. BLE Implementation Feedback (PR #4714)

The issues you've identified with the BLE implementation are common pain points in cross-platform mobile Bluetooth development.

  1. Excessive Logging (Too Verbose)

    Diagnosis: Verbose logging, while useful for initial debugging, can severely impact performance, especially on Android. Log buffer exhaustion, repeated I/O calls to the logcat, and CPU cycles spent formatting strings can introduce unnecessary latency.

    Recommendation:

     Implement log levels in your native code (e.g., Kotlin/Java).
    
     By default, set the log level to WARNING or ERROR.
    
     Only enable DEBUG or VERBOSE logging via a CLI setting or a developer option in the Configurator that sets a preference flag read by the native plugin. This keeps the production app fast but allows for deep debugging when needed.
    
  2. Slow Read/Write Performance (Refactoring)

The performance issues are likely due to the inherent nature of how BLE transactions are queued on the native side and the low default throughput.

Refactoring Strategy (Read/Write Separation):

    Goal: Don't have one native method that tries to do both. A read() should only read; a write() should only write.

    Write Optimization (The Priority): The search results confirm that Android's BLE stack queues writes and often waits for the onCharacteristicWrite callback before sending the next one.

        Solution A (Flow Control): Implement a Write Queue in your native code. Do not call writeCharacteristic() for the next chunk until the onCharacteristicWrite() callback for the previous chunk has been received. This ensures reliability and prevents the OS from discarding writes, but it maintains the inherent serial delay.

        Solution B (Maximum Throughput): Use BluetoothGatt.requestMtu(256) or higher immediately after connecting. Increasing the Maximum Transmission Unit (MTU) allows you to send larger data packets, dramatically reducing the number of write operations and thus minimizing the queuing delay.

        Solution C (Best Effort): If the Betaflight FC supports it, set the Write Type to WRITE_TYPE_NO_RESPONSE for high-frequency MSP data that is not critical (like fast telemetry). This bypasses the wait for the ACK, but data loss is possible.

Read Optimization: Use Notifications (setCharacteristicNotification(true)). This is an asynchronous, push-based model. It eliminates polling (read()) and is far more efficient for the continuous MSP telemetry stream. Your native plugin's event loop should wait for the onCharacteristicChanged callback, which then triggers notifyListeners to the JavaScript side.
  1. Missing Permission/Device Selection Dialog

The device discovery and selection dialog must be handled explicitly in your plugin to give the user control.

  • Recommended Flow (Using requestDevice() pattern):
    • Check Permissions: Before scanning, your plugin must check for BLUETOOTH_SCAN and ACCESS_FINE_LOCATION (pre-Android 12) or BLUETOOTH_CONNECT (Android 12+).
    • Request Device/Scan: Use the native equivalent of a standard BLE client's requestDevice(options) (as noted in search results 4.1).
    • Implement Native Dialog: Your plugin needs to wrap the native scanning process. Instead of simply returning a list of devices, the native code should start the scan and then display a modal/dialog within the Android Activity to let the user select the FC.
    • Resolve on Selection: Only when the user selects a device from your custom dialog does the native plugin resolve the promise back to JavaScript with the deviceId. This is the user-friendly approach required for production apps.

For more details on why your read/write speed may be limited and how to implement flow control, you can review this video: Android Bluetooth Low Energy Sequential Write Performance. This video discusses the typical write limitations in the Android BLE stack.

@coderabbitai perhaps take a deep dive and suggest changes for each step.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Tip

For best results, initiate chat on the files or code changes.

@haslinghuis here’s a focused, step-by-step plan with minimal patches for each item. I’ve kept write() strictly write-only, moved all decisions to connect-time, and avoided adding unrelated helpers.

A) Logging is too verbose (Android + JS)

Android (BetaflightBluetoothPlugin.java)

  • Add a tiny log-level gate and a setter. Default WARN in release; DEBUG only when you flip it from JS.
// Fields near other members
private static final String TAG = "BetaflightBluetooth";
private static volatile int LOG_LEVEL = android.util.Log.WARN; // VERBOSE<DEBUG<INFO<WARN<ERROR

@PluginMethod
public void setLogLevel(PluginCall call) {
    String level = call.getString("level", "warn").toLowerCase(Locale.ROOT);
    switch (level) {
        case "verbose": LOG_LEVEL = Log.VERBOSE; break;
        case "debug":   LOG_LEVEL = Log.DEBUG;   break;
        case "info":    LOG_LEVEL = Log.INFO;    break;
        case "error":   LOG_LEVEL = Log.ERROR;   break;
        default:        LOG_LEVEL = Log.WARN;
    }
    JSObject ret = new JSObject(); ret.put("level", level); call.resolve(ret);
}

private static void log(int lvl, String msg) {
    if (lvl >= LOG_LEVEL) Log.println(lvl, TAG, msg);
}
  • Replace noisy Log.d/w statements in hot paths with log(Log.DEBUG, "..."). No behavior change; just gated.

JS (src/js/protocols/CapacitorBluetooth.js)

  • Gate logs with a local flag so production builds stay quiet; devs can enable ad-hoc.
// Top of file
const DBG_BT = !!(window?.BF_DEBUG_BLE); // set via console or dev toggle

function dbg(...args) { if (DBG_BT) console.log("[BLE]", ...args); }

// Replace verbose console.log(...) calls with dbg(...)

Optionally set Android log level at app start:

try { BetaflightBluetooth.setLogLevel({ level: "warn" }); } catch {}

B) Read/write is slow — refactor to “write-only” and do all setup once on connect

Android (BetaflightBluetoothPlugin.java)

  • Cache targets and write type exactly once during service discovery. Make write() decode+submit only. Reset caches on all disconnect paths.
  1. Fields
// Cached per connection
private BluetoothGattCharacteristic writeCharacteristic = null;
private boolean writeNoResponseSupported = false;
private int cachedWriteType = BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
  1. onConnectionStateChange → after STATE_CONNECTED, do “once” optimizations
@Override
public void onConnectionStateChange(BluetoothGatt gatt, int newState, int status) {
    if (newState == BluetoothProfile.STATE_CONNECTED) {
        // Optional throughput boosters (safe no-ops on older APIs)
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
            try { gatt.setPreferredPhy(BluetoothDevice.PHY_LE_2M, BluetoothDevice.PHY_LE_2M,
                    BluetoothDevice.PHY_OPTION_NO_PREFERRED); } catch (Exception ignored) {}
        }
        try { gatt.requestMtu(247); } catch (Exception ignored) {} // 247 is broadly supported
        gatt.discoverServices();
        return;
    }
    if (newState == BluetoothProfile.STATE_DISCONNECTED) {
        cleanupGatt();
        notifyConnectionState("disconnected", null);
    }
}
  1. onServicesDiscovered → resolve once, prefer WRITE_NO_RESPONSE, compute cachedWriteType
@Override
public void onServicesDiscovered(BluetoothGatt gatt, int status) {
    if (status != BluetoothGatt.GATT_SUCCESS) return;
    servicesDiscovered = true;

    writeCharacteristic = null;
    writeNoResponseSupported = false;
    cachedWriteType = BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;

    for (BluetoothGattService svc : gatt.getServices()) {
        BluetoothGattCharacteristic preferred = null, fallback = null;
        for (BluetoothGattCharacteristic ch : svc.getCharacteristics()) {
            int props = ch.getProperties();
            if ((props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0) { preferred = ch; break; }
            if (fallback == null && (props & BluetoothGattCharacteristic.PROPERTY_WRITE) != 0) fallback = ch;
        }
        BluetoothGattCharacteristic chosen = (preferred != null) ? preferred : fallback;
        if (chosen != null) {
            writeCharacteristic = chosen;
            writeNoResponseSupported =
                (chosen.getProperties() & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0;
            cachedWriteType = writeNoResponseSupported
                ? BluetoothGattCharacteristic.WRITE_TYPE_NO_RESPONSE
                : BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
            break;
        }
    }

    flushPendingStartNotificationCalls();
    // ... existing “services” event notification stays as-is ...
}
  1. write() stays minimal (decode → submitWrite with cachedWriteType)
@PluginMethod
public void write(PluginCall call) {
    if (!ensureConnected(call)) return;
    final BluetoothGatt gatt = bluetoothGatt;
    if (gatt == null || !servicesDiscovered) { call.reject("Not connected"); return; }

    final BluetoothGattCharacteristic target = writeCharacteristic;
    if (target == null) { call.reject("Write characteristic not available"); return; }

    final String value = call.getString("value", call.getString("data"));
    final String encoding = call.getString("encoding", "base64");
    final byte[] payload;
    try { payload = decodePayload(value, encoding); }
    catch (IllegalArgumentException ex) { call.reject("Failed to decode payload: " + ex.getMessage()); return; }

    final boolean ok = submitWrite(gatt, target, payload, cachedWriteType);
    if (ok) {
        JSObject res = new JSObject(); res.put("bytesSent", payload.length); call.resolve(res);
    } else {
        call.reject("Unable to write characteristic");
    }
}
  1. cleanupGatt() must reset everything (called on all disconnect cases)
private void cleanupGatt() {
    if (bluetoothGatt != null) {
        try { bluetoothGatt.disconnect(); } catch (Exception ignored) {}
        try { bluetoothGatt.close(); } catch (Exception ignored) {}
    }
    bluetoothGatt = null;
    writeCharacteristic = null;
    writeNoResponseSupported = false;
    cachedWriteType = BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
    connectedDeviceId = null;
    servicesDiscovered = false;
}

JS (src/js/protocols/CapacitorBluetooth.js)

  • Prevent the early “Unable to write characteristic” right after reboot by gating “open” until native services are ready, plus a one-shot retry on that exact error. This keeps MSP traffic from firing too soon.
// Wait for the native 'services' event for this device before announcing 'open'
_waitForServices(deviceId, timeoutMs = 4000) {
  if (this._servicesReady) return Promise.resolve();
  return new Promise((resolve, reject) => {
    const timer = setTimeout(() => reject(new Error("Services discovery timeout")), timeoutMs);
    const sub = BetaflightBluetooth.addListener("services", (payload) => {
      if (!payload || payload.deviceId !== deviceId) return;
      clearTimeout(timer); this._servicesReady = true; sub.remove(); resolve();
    });
  });
}

async connect(device, options = {}) {
  // ... existing connect ...
  await this._waitForServices(device.deviceId, 4000);
  await BetaflightBluetooth.startNotifications({ deviceId: device.deviceId });

  this.dispatchEvent(new CustomEvent("open", {
    detail: { path: "bluetooth", baudRate: options.baudRate ?? 115200 }
  }));
}

Optional one-shot retry in send():

async send(buffer) {
  const data = buffer instanceof Uint8Array ? buffer : new Uint8Array(buffer);
  try {
    await BetaflightBluetooth.write({ value: arrayBufferToBase64(data), encoding: "base64" });
    this.bytesSent += data.length;
  } catch (err) {
    if (!this._writeRetried && String(err?.message || err).includes("Unable to write characteristic")) {
      this._writeRetried = true;
      await new Promise(r => setTimeout(r, 60));
      try { await this._waitForServices(this.device?.device?.deviceId ?? this.device?.deviceId, 2000); } catch {}
      return this.send(data);
    }
    this._writeRetried = false;
    throw err;
  }
  this._writeRetried = false;
}

C) No dialog when permissions are required to choose a device (native picker in requestDevice)

  • Implement a simple native device picker (AlertDialog). It:
    1. Verifies permissions (API 26–30: BLUETOOTH/ADMIN + COARSE_LOCATION; API 31+: SCAN/CONNECT).
    2. Starts LE scan (accept-all; don’t filter by service to match WebBluetooth semantics).
    3. Shows a single-choice dialog with device names + RSSI.
    4. On selection: stop scan and resolve { deviceId, name, uuids } to JS.
    5. On cancel: stop scan and reject.

Skeleton (drop-in for your @PluginMethod requestDevice):

@PluginMethod
public void requestDevice(PluginCall call) {
    Activity activity = getActivity();
    if (activity == null) { call.reject("No activity"); return; }

    // 1) Ensure permissions by API level (per your manifest split)
    if (!ensurePermissionsForScan(call)) return; // implement using your existing permission helpers

    activity.runOnUiThread(() -> {
        // 2) Start scanning and collect results (accept all; fill discoveredDevices list)
        startLeScan(/* accept-all, no filters */);

        // 3) Build dialog list from current discovered devices
        final List<BluetoothDevice> list = new ArrayList<>(discoveredDevices.values());
        final ArrayAdapter<String> adapter = new ArrayAdapter<>(activity, android.R.layout.select_dialog_singlechoice);
        for (BluetoothDevice d : list) adapter.add(d.getName() != null ? d.getName() : d.getAddress());

        new AlertDialog.Builder(activity)
            .setTitle("Select Bluetooth device")
            .setSingleChoiceItems(adapter, -1, null)
            .setPositiveButton("Select", (dlg, which) -> {
                ListView lv = ((AlertDialog) dlg).getListView();
                int idx = lv.getCheckedItemPosition();
                if (idx >= 0 && idx < list.size()) {
                    BluetoothDevice dev = list.get(idx);
                    stopLeScan();
                    JSObject ret = new JSObject();
                    ret.put("deviceId", dev.getAddress());
                    ret.put("name", dev.getName());
                    // If you maintain uuids in discoveredDevices, include them:
                    // ret.put("uuids", toJsArray(knownUuidsFor(dev)));
                    call.resolve(ret);
                } else {
                    stopLeScan(); call.reject("No device selected");
                }
            })
            .setNegativeButton("Cancel", (dlg, w) -> { stopLeScan(); call.reject("Cancelled"); })
            .setOnCancelListener(dlg -> { stopLeScan(); call.reject("Cancelled"); })
            .show();
    });
}

Notes

  • Keep scanning accept-all; match PWA’s “acceptAllDevices + optionalServices” so users can see everything that’s advertising.
  • Reuse your existing permission utility (per learning: version-aware split). Don’t request non-existent permissions on older Android.
  • Port handler flow in JS can first try getDevices(); if empty, call requestDevice() to pop this native picker.

D) Ensure Save & Reboot always reaches reinitializeConnection (BLE-safe)

  • With the above transport fixes, the MSP_EEPROM_WRITE ack should arrive reliably. As a belt-and-suspenders, keep the tiny MSP-side fallback so UX can’t dead-end on noisier Android stacks:
// src/js/msp/MSPHelper.js — before EEPROM write:
try { GUI.interval_kill_all(); MSP.callbacks_cleanup(); } catch {}
let finished = false;
const finish = () => { if (finished) return; finished = true;
  if (reboot) GUI.tab_switch_cleanup(() => reinitializeConnection());
  if (callback) callback();
};
const fb = setTimeout(() => { console.warn("EEPROM ack timeout; proceeding to reboot"); finish(); }, 3000);

MSP.send_message(MSPCodes.MSP_EEPROM_WRITE, false, false, () => {
  clearTimeout(fb); gui_log(i18n.getMessage("configurationEepromSaved")); finish();
});

Summary of what changes where

  • Android plugin:

    • setLogLevel() + log gate (defaults WARN).
    • onConnectionStateChange: requestMtu + setPreferredPhy.
    • onServicesDiscovered: resolve writeCharacteristic once; compute cachedWriteType once.
    • write(): decode+submit only (no UUID parsing; no property reads).
    • cleanupGatt(): reset caches (writeCharacteristic / cachedWriteType / flags) on all disconnects.
    • requestDevice(): native picker dialog (accept-all scan, version-aware permissions).
  • JS:

    • Gate ‘open’ on native ‘services’ event; start notifications before open.
    • Optional one-shot retry in send() on transient “Unable to write characteristic”.
    • Logging guard to reduce console noise.
    • (MSP) Keep the small fallback so reboot flow can’t get stuck.

If you want, I can turn this into a small follow-up patch that only touches:

  • BetaflightBluetoothPlugin.java (log-level, MTU/PHY, caches, write-only, cleanup, picker),
  • CapacitorBluetooth.js (gated open + optional retry + log guard),
  • MSPHelper.js (3s fallback as shown).

Which parts would you like me to prepare first for a CI build?


🧠 Learnings used
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:38:41.581Z
Learning: On BLE (Android) Save & Reboot, status pull intervals continue during MSP_EEPROM_WRITE, overwhelming the MSP queue and causing the EEPROM ack to time out so reinitializeConnection never runs. Solution: before EEPROM write, call GUI.interval_kill_all() and MSP.callbacks_cleanup(), and add a 3s fallback to proceed to reboot if the ack is missed.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: In betaflight-configurator’s CapacitorBluetooth.js, native plugin listeners (notification, services, connectionState) must be attached exactly once per app lifetime. Re-attaching them during writes/connect cycles leads to duplicate handlers, increased JS workload, MSP queue pressure, and can prevent the MSP_EEPROM_WRITE callback from firing (so reinitializeConnection is not reached). Implement an idempotent _attachNativeListenersOnce() guard.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: CapacitorBluetooth.connect should only dispatch the 'open' event after GATT services are resolved and notifications are started; announcing open earlier causes the first MSP write to fail with “Unable to write characteristic” and contributes to early queue load.

Learnt from: ctzsnooze
Repo: betaflight/betaflight-configurator PR: 4627
File: src/js/tabs/onboard_logging.js:504-506
Timestamp: 2025-09-28T12:56:15.659Z
Learning: The Betaflight Configurator MSP queue can become overwhelmed during dataflash reads when retry logic is used, even with minimal retries (MAX_SIMPLE_RETRIES = 1), because failed blocks cause exponential queue growth. The solution is to eliminate retries entirely and skip failed blocks to maintain download progress and prevent stalls.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/tabs/pid_tuning.js:80-82
Timestamp: 2025-08-27T19:36:13.733Z
Learning: MSP_STATUS_EX calls in serial_backend.js and setup.js are intentionally unguarded because they are used for status fetching on page load to initialize profiles and are safe to call on older firmware versions. Only conditional/optional MSP_STATUS_EX usage (like in pid_tuning.js) needs version gating to API_VERSION_25_12.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:33:22.959Z
Learning: In the betaflight-configurator codebase, port paths use counter suffixes for serial and bluetooth ports (e.g., "serial1", "serial2", "bluetooth1", "bluetooth2") instead of using connectionId, which means simple string matching against protocol map keys won't work for protocol selection.

Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4583
File: src/js/tabs/firmware_flasher.js:949-961
Timestamp: 2025-09-02T07:45:48.606Z
Learning: In src/js/tabs/firmware_flasher.js, the config file loading code path after firmware loading (in the load_file click handler) cannot be reached when UF2 firmware is loaded, according to the maintainer blckmn. This code is maintained for backward compatibility with unified target settings and will be refactored in a future PR.

Learnt from: ctzsnooze
Repo: betaflight/betaflight-configurator PR: 4627
File: src/js/FileSystem.js:101-107
Timestamp: 2025-09-28T16:39:04.171Z
Learning: In the Betaflight Configurator dataflash download implementation, removing await from FileSystem.writeChunk() and closeFile() is intentional to prevent MSP queue overwhelm. Awaiting file I/O operations slows the download loop enough to cause MSP timeouts and queue growth, leading to stalls. The non-blocking approach prioritizes download throughput over immediate file write error detection.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-09-01T11:01:38.695Z
Learning: The Betaflight Configurator implements analytics through src/js/Analytics.js with the URL "https://analytics.betaflight.com" at line 68, which serves as a proxy frontend for the actual telemetry provider (currently Telemetry Deck), allowing vendor flexibility without code changes.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-02T17:46:47.859Z
Learning: BetaflightBluetoothPlugin.java: maintainers prefer write(PluginCall) to be “write-only” — no per-call UUID parsing or characteristic lookups. Resolve and cache the write characteristic once in onServicesDiscovered() (prefer PROPERTY_WRITE_NO_RESPONSE; else PROPERTY_WRITE). write() should read payload params from call, use the cached writeCharacteristic, pick write type, and submitWrite; clear the cache in cleanupGatt().

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.784Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Tip

For best results, initiate chat on the files or code changes.

@haslinghuis here’s a focused, minimal set of changes addressing the three items (logging, read/write speed, device-selection dialog), keeping write() strictly “write-only” and moving all decisions to connect-time. I’ve aligned with your latest write() structure and avoided adding unrelated helpers.

A) Tone down logging (default WARN; switchable at runtime)
android/app/src/main/java/betaflight/configurator/protocols/bluetooth/BetaflightBluetoothPlugin.java

  • Add a tiny gate and a PluginMethod to flip level on demand. Replace hot-path Log.* calls with log(…) as you see fit.
// near top
private static final String TAG = "BetaflightBluetooth";
private static volatile int LOG_LEVEL = android.util.Log.WARN; // VERBOSE<DEBUG<INFO<WARN<ERROR
private static void log(int lvl, String msg) { if (lvl >= LOG_LEVEL) android.util.Log.println(lvl, TAG, msg); }

// Optional: expose to JS so QA can turn up logging when needed
@PluginMethod
public void setLogLevel(PluginCall call) {
    String level = String.valueOf(call.getString("level", "warn")).toLowerCase(Locale.ROOT);
    switch (level) {
        case "verbose": LOG_LEVEL = Log.VERBOSE; break;
        case "debug":   LOG_LEVEL = Log.DEBUG;   break;
        case "info":    LOG_LEVEL = Log.INFO;    break;
        case "error":   LOG_LEVEL = Log.ERROR;   break;
        default:        LOG_LEVEL = Log.WARN;
    }
    JSObject ret = new JSObject(); ret.put("level", level); call.resolve(ret);
}

JS usage (optional, once on app init):

try { BetaflightBluetooth.setLogLevel({ level: "warn" }); } catch {}

B) Make write fast: resolve everything once; keep write “decode → submit”

  1. Cache write characteristic and write type once in onServicesDiscovered()
  2. Keep write() minimal (no UUID parsing, no GATT/property reads)
  3. Reset caches on any disconnect/cleanup

BetaflightBluetoothPlugin.java

// class fields (near other members)
private BluetoothGattCharacteristic writeCharacteristic = null;
private boolean writeNoResponseSupported = false;
private int cachedWriteType = BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;

// onConnectionStateChange (optional throughput boosts; safe no-ops on some stacks)
@Override
public void onConnectionStateChange(BluetoothGatt gatt, int newState, int status) {
    if (newState == BluetoothProfile.STATE_CONNECTED) {
        try { gatt.requestMtu(247); } catch (Exception ignored) {}
        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
            try { gatt.setPreferredPhy(BluetoothDevice.PHY_LE_2M, BluetoothDevice.PHY_LE_2M,
                    BluetoothDevice.PHY_OPTION_NO_PREFERRED); } catch (Exception ignored) {}
        }
        gatt.discoverServices();
        return;
    }
    if (newState == BluetoothProfile.STATE_DISCONNECTED) {
        cleanupGatt();
        notifyConnectionState("disconnected", null);
    }
}

// onServicesDiscovered: resolve once, prefer WRITE_NO_RESPONSE
@Override
public void onServicesDiscovered(BluetoothGatt gatt, int status) {
    if (status != BluetoothGatt.GATT_SUCCESS) return;
    servicesDiscovered = true;

    writeCharacteristic = null;
    writeNoResponseSupported = false;
    cachedWriteType = BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;

    for (BluetoothGattService svc : gatt.getServices()) {
        BluetoothGattCharacteristic preferred = null, fallback = null;
        for (BluetoothGattCharacteristic ch : svc.getCharacteristics()) {
            int props = ch.getProperties();
            if ((props & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0) {
                preferred = ch; break;
            }
            if (fallback == null && (props & BluetoothGattCharacteristic.PROPERTY_WRITE) != 0) {
                fallback = ch;
            }
        }
        final BluetoothGattCharacteristic chosen = (preferred != null) ? preferred : fallback;
        if (chosen != null) {
            writeCharacteristic = chosen;
            writeNoResponseSupported =
                (chosen.getProperties() & BluetoothGattCharacteristic.PROPERTY_WRITE_NO_RESPONSE) != 0;
            cachedWriteType = writeNoResponseSupported
                ? BluetoothGattCharacteristic.WRITE_TYPE_NO_RESPONSE
                : BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
            break;
        }
    }

    flushPendingStartNotificationCalls(); // keep your existing call
    // ... existing “services” payload emission remains unchanged ...
}

// write() — decode + submit only (no per-call lookups, no property reads)
@PluginMethod
public void write(PluginCall call) {
    if (!ensureConnected(call)) return;

    final BluetoothGatt gatt = bluetoothGatt;
    if (gatt == null || !servicesDiscovered) {
        call.reject("Not connected");
        return;
    }
    final BluetoothGattCharacteristic target = writeCharacteristic;
    if (target == null) {
        call.reject("Write characteristic not available");
        return;
    }

    final String value = call.getString("value", call.getString("data"));
    final String encoding = call.getString("encoding", "base64");
    final byte[] payload;
    try {
        payload = decodePayload(value, encoding); // reuse your existing helper
    } catch (IllegalArgumentException ex) {
        call.reject("Failed to decode payload: " + ex.getMessage());
        return;
    }

    final boolean ok = submitWrite(gatt, target, payload, cachedWriteType);
    if (ok) {
        JSObject result = new JSObject();
        result.put("bytesSent", payload.length);
        call.resolve(result);
    } else {
        call.reject("Unable to write characteristic");
    }
}

// cleanup — must clear caches on any disconnect path
private void cleanupGatt() {
    if (bluetoothGatt != null) {
        try { bluetoothGatt.disconnect(); } catch (Exception ignored) {}
        try { bluetoothGatt.close(); } catch (Exception ignored) {}
    }
    bluetoothGatt = null;
    writeCharacteristic = null;
    writeNoResponseSupported = false;
    cachedWriteType = BluetoothGattCharacteristic.WRITE_TYPE_DEFAULT;
    connectedDeviceId = null;
    servicesDiscovered = false;
}

Small JS guard to prevent “Unable to write characteristic” around reboot

  • Wait for native “services” before announcing 'open'
  • Start notifications before 'open'
  • Optional one-shot retry for the exact error string

src/js/protocols/CapacitorBluetooth.js

// helper
_waitForServices(deviceId, timeoutMs = 4000) {
  if (this._servicesReady) return Promise.resolve();
  return new Promise((resolve, reject) => {
    const timer = setTimeout(() => reject(new Error("Services discovery timeout")), timeoutMs);
    const sub = BetaflightBluetooth.addListener("services", (payload) => {
      if (!payload || payload.deviceId !== deviceId) return;
      clearTimeout(timer);
      this._servicesReady = true;
      sub.remove();
      resolve();
    });
  });
}

async connect(device, options = {}) {
  // ... your existing connect() up to native connect(...)
  await this._waitForServices(device.deviceId, 4000);
  await BetaflightBluetooth.startNotifications({ deviceId: device.deviceId });

  // now emit 'open'
  this.dispatchEvent(new CustomEvent("open", {
    detail: { path: "bluetooth", baudRate: options.baudRate ?? 115200 }
  }));
}

// optional, very small one-shot retry
async send(buffer) {
  const data = buffer instanceof Uint8Array ? buffer : new Uint8Array(buffer);
  try {
    await BetaflightBluetooth.write({ value: arrayBufferToBase64(data), encoding: "base64" });
    this.bytesSent += data.length;
  } catch (err) {
    const msg = String(err?.message || err);
    if (!this._writeRetried && msg.includes("Unable to write characteristic")) {
      this._writeRetried = true;
      await new Promise(r => setTimeout(r, 60));
      try { await this._waitForServices(this.device?.device?.deviceId ?? this.device?.deviceId, 2000); } catch {}
      return this.send(data);
    }
    this._writeRetried = false;
    throw err;
  }
  this._writeRetried = false;
}

C) Device-selection dialog (native requestDevice)
Pattern: version-aware permission check → start LE scan (accept-all) → show AlertDialog with names/RSSI → resolve on selection → stop scan on any exit. This mirrors the WebBluetooth requestDevice UX and unblocks multi-device environments.

BetaflightBluetoothPlugin.java (skeleton that plugs into your existing scan map)

@PluginMethod
public void requestDevice(PluginCall call) {
    Activity activity = getActivity();
    if (activity == null) { call.reject("No activity"); return; }

    // Ensure permissions per API level (use your existing helpers)
    if (!ensurePermissionsForScan(call)) return;

    activity.runOnUiThread(() -> {
        // 1) start accept-all scan (reuse your current startScan mechanism)
        try { startScanAcceptAll(); } catch (Exception e) { call.reject("Scan failed: " + e.getMessage()); return; }

        // 2) build a list from your discoveredDevices map
        final List<BluetoothDevice> devices = new ArrayList<>(discoveredDevices.values());
        final ArrayAdapter<String> adapter =
            new ArrayAdapter<>(activity, android.R.layout.select_dialog_singlechoice);
        for (BluetoothDevice d : devices) {
            String name = (d.getName() != null) ? d.getName() : d.getAddress();
            adapter.add(name);
        }

        // 3) dialog
        AlertDialog dlg = new AlertDialog.Builder(activity)
            .setTitle("Select Bluetooth device")
            .setSingleChoiceItems(adapter, -1, null)
            .setPositiveButton("Select", (dialog, whichBtn) -> {
                ListView lv = ((AlertDialog) dialog).getListView();
                int idx = lv.getCheckedItemPosition();
                stopScanSafe();
                if (idx >= 0 && idx < devices.size()) {
                    BluetoothDevice dev = devices.get(idx);
                    JSObject ret = new JSObject();
                    ret.put("deviceId", dev.getAddress());
                    ret.put("name", dev.getName());
                    // If you keep uuids per device, add them as a JSArray here.
                    call.resolve(ret);
                } else {
                    call.reject("No device selected");
                }
            })
            .setNegativeButton("Cancel", (d, w) -> { stopScanSafe(); call.reject("Cancelled"); })
            .setOnCancelListener(d -> { stopScanSafe(); call.reject("Cancelled"); })
            .create();
        dlg.show();
    });
}

// helpers using your existing scan API
private void startScanAcceptAll() { /* reuse your scanner with no filters */ }
private void stopScanSafe() { try { stopScanInternal(); } catch (Exception ignored) {} }

JS side (use the native picker when we don’t already have a permitted device)

// When user clicks “Connect via Bluetooth”
let dev = null;
try {
  dev = await BetaflightBluetooth.requestDevice(); // opens native picker
} catch (e) {
  // user cancelled or permissions missing
  // optionally fall back to getDevices() or show a UI message
}
if (dev) {
  await serial.connect("bluetooth", { device: dev, baudRate: 115200 });
}

Sequence (end-to-end)

sequenceDiagram
  autonumber
  participant UI as UI
  participant JS as CapacitorBluetooth.js
  participant Plugin as BetaflightBluetoothPlugin
  participant GATT as Android BLE Stack

  UI->>JS: requestDevice()
  JS->>Plugin: requestDevice()
  Plugin->>GATT: startScan (accept all)
  GATT-->>Plugin: onScanResult(...)
  Plugin-->>JS: (dialog) resolve { deviceId, name }
  JS->>Plugin: connect(deviceId)
  Plugin->>GATT: connect + requestMtu + discoverServices
  GATT-->>Plugin: onServicesDiscovered
  Plugin-->>JS: services event
  JS->>Plugin: startNotifications
  JS-->>UI: dispatch 'open'
  UI->>JS: MSP writes
  JS->>Plugin: write(value)
  Plugin->>GATT: writeCharacteristic (cachedWriteType)
  GATT-->>Plugin: onCharacteristicChanged (notifications)
  Plugin-->>JS: notification event
Loading

Why this resolves your three issues

  • Logging: noise reduced to near-zero in production; still switchable for diagnostics.
  • Speed: write() is now constant-time (decode + submit); characteristic and write type are resolved exactly once; caches are cleared decisively on disconnect to avoid stale handles after reboot.
  • Device selection: native picker provides a clear, user-friendly selection flow with proper permission gating on Android 8–11 vs 12+.

Sanity checks to run while testing on Android 8 and 15

  • Connect: no “Unable to write characteristic” after we gate ‘open’ on services; if it appears once, the one-shot retry masks it.
  • Save & Reboot (Ports/Configuration/Failsafe/GPS/Motors/Onboard Logging):
    • With the earlier MSP-side quiet + fallback in MSPHelper, reinitializeConnection should always run (either on EEPROM ack or via timeout fallback).
  • Disconnect/reboot: verify writeCharacteristic and cachedWriteType are null after DISCONNECTED and repopulated after the next onServicesDiscovered.

If you want, I can turn this into a tiny follow-up patch (only these deltas in BetaflightBluetoothPlugin.java and CapacitorBluetooth.js) so you can grab a CI artifact and confirm on real hardware.


✏️ Learnings added
Learnt from: haslinghuis
URL: https://github.com/betaflight/betaflight-configurator/pull/4714

Timestamp: 2025-12-10T17:48:24.215Z
Learning: BetaflightBluetoothPlugin.java: compute and cache write type (cachedWriteType) once in onServicesDiscovered() and keep write(PluginCall) strictly “write-only” (decode + submitWrite using cachedWriteType); clear caches in cleanupGatt() on every disconnect.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:38:41.581Z
Learning: On BLE (Android) Save & Reboot, status pull intervals continue during MSP_EEPROM_WRITE, overwhelming the MSP queue and causing the EEPROM ack to time out so reinitializeConnection never runs. Solution: before EEPROM write, call GUI.interval_kill_all() and MSP.callbacks_cleanup(), and add a 3s fallback to proceed to reboot if the ack is missed.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: In betaflight-configurator’s CapacitorBluetooth.js, native plugin listeners (notification, services, connectionState) must be attached exactly once per app lifetime. Re-attaching them during writes/connect cycles leads to duplicate handlers, increased JS workload, MSP queue pressure, and can prevent the MSP_EEPROM_WRITE callback from firing (so reinitializeConnection is not reached). Implement an idempotent _attachNativeListenersOnce() guard.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-01T17:09:13.412Z
Learning: CapacitorBluetooth.connect should only dispatch the 'open' event after GATT services are resolved and notifications are started; announcing open earlier causes the first MSP write to fail with “Unable to write characteristic” and contributes to early queue load.

Learnt from: ctzsnooze
Repo: betaflight/betaflight-configurator PR: 4627
File: src/js/tabs/onboard_logging.js:504-506
Timestamp: 2025-09-28T12:56:15.659Z
Learning: The Betaflight Configurator MSP queue can become overwhelmed during dataflash reads when retry logic is used, even with minimal retries (MAX_SIMPLE_RETRIES = 1), because failed blocks cause exponential queue growth. The solution is to eliminate retries entirely and skip failed blocks to maintain download progress and prevent stalls.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/tabs/pid_tuning.js:80-82
Timestamp: 2025-08-27T19:36:13.733Z
Learning: MSP_STATUS_EX calls in serial_backend.js and setup.js are intentionally unguarded because they are used for status fetching on page load to initialize profiles and are safe to call on older firmware versions. Only conditional/optional MSP_STATUS_EX usage (like in pid_tuning.js) needs version gating to API_VERSION_25_12.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:47-47
Timestamp: 2025-11-24T15:07:25.227Z
Learning: In betaflight-configurator's CapacitorSerial protocol (src/js/protocols/CapacitorSerial.js), the fire-and-forget this.loadDevices() call in the constructor is intentional background pre-loading. The actual critical device list loading for UI population is orchestrated by port_handler.js via serial.getDevices("serial"), which properly awaits CapacitorSerial.getDevices() → loadDevices() in a complete async chain. The constructor's async call does not cause race conditions.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-28T22:51:11.691Z
Learning: In betaflight-configurator, Web Bluetooth API's `services` option filters scan results to only devices advertising those UUIDs, while `optionalServices` requests permission to access services after connection without filtering. PWA (WebBluetooth.js) uses `{ acceptAllDevices: true, optionalServices: uuids }` to show all devices while requesting post-connection service access. CapacitorBluetooth.js should match this pattern rather than using `services` as a scan filter.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:135-143
Timestamp: 2025-11-28T22:41:59.374Z
Learning: In betaflight-configurator, WebSerial.js uses a constant path "serial" for all devices, and CapacitorBluetooth.js uses constant path "bluetooth", consistent with the single-device-at-a-time workflow. WebBluetooth.js uses counter-based paths (bluetooth_${counter}), but that's specific to its implementation. The path strategy varies by protocol based on their specific requirements.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4706
File: src/js/protocols/CapacitorSerial.js:11-46
Timestamp: 2025-11-22T21:18:08.814Z
Learning: In betaflight-configurator, protocol implementations (WebSerial, CapacitorSerial, WebBluetooth, etc.) are instantiated once in the Serial class constructor and stored as singletons in the _protocols array for the entire application lifetime. They are never destroyed or recreated, so cleanup methods to remove event listeners are not needed.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:32:21.385Z
Learning: In the betaflight-configurator codebase, port paths use counter prefixes (e.g., "bluetooth1", "bluetooth2", "serial1") rather than direct protocol identifiers. The protocol selection logic correctly uses `portPath.startsWith("bluetooth")` to detect bluetooth ports regardless of the counter suffix, rather than direct string matching against protocol map keys.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:33:22.959Z
Learning: In the betaflight-configurator codebase, port paths use counter suffixes for serial and bluetooth ports (e.g., "serial1", "serial2", "bluetooth1", "bluetooth2") instead of using connectionId, which means simple string matching against protocol map keys won't work for protocol selection.

Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4583
File: src/js/tabs/firmware_flasher.js:949-961
Timestamp: 2025-09-02T07:45:48.606Z
Learning: In src/js/tabs/firmware_flasher.js, the config file loading code path after firmware loading (in the load_file click handler) cannot be reached when UF2 firmware is loaded, according to the maintainer blckmn. This code is maintained for backward compatibility with unified target settings and will be refactored in a future PR.

Learnt from: ctzsnooze
Repo: betaflight/betaflight-configurator PR: 4627
File: src/js/FileSystem.js:101-107
Timestamp: 2025-09-28T16:39:04.171Z
Learning: In the Betaflight Configurator dataflash download implementation, removing await from FileSystem.writeChunk() and closeFile() is intentional to prevent MSP queue overwhelm. Awaiting file I/O operations slows the download loop enough to cause MSP timeouts and queue growth, leading to stalls. The non-blocking approach prioritizes download throughput over immediate file write error detection.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4532
File: src/js/VirtualFC.js:234-234
Timestamp: 2025-06-27T22:06:49.210Z
Learning: In the betaflight-configurator codebase, the VirtualFC.js AUX_CONFIG array must stay synchronized with the betaflight firmware's msp_box.c definitions to ensure proper auxiliary mode functionality. Changes to mode names should follow the firmware source code even if it potentially breaks backward compatibility with saved presets.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:51:09.253Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported for BLE devices and Web USB API is supported, but Web Serial API is not supported (except limited Bluetooth serial support added in 2025). The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-09-01T11:01:38.695Z
Learning: The Betaflight Configurator implements analytics through src/js/Analytics.js with the URL "https://analytics.betaflight.com" at line 68, which serves as a proxy frontend for the actual telemetry provider (currently Telemetry Deck), allowing vendor flexibility without code changes.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-02T17:46:47.859Z
Learning: BetaflightBluetoothPlugin.java: maintainers prefer write(PluginCall) to be “write-only” — no per-call UUID parsing or characteristic lookups. Resolve and cache the write characteristic once in onServicesDiscovered() (prefer PROPERTY_WRITE_NO_RESPONSE; else PROPERTY_WRITE). write() should read payload params from call, use the cached writeCharacteristic, pick write type, and submitWrite; clear the cache in cleanupGatt().

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4714
File: src/js/protocols/CapacitorBluetooth.js:176-206
Timestamp: 2025-11-30T18:10:26.784Z
Learning: In betaflight-configurator's Android Bluetooth implementation, permission handling must be version-aware: Android 8-11 (API 26-30) requires BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION permissions, while Android 12+ (API 31+) requires BLUETOOTH_SCAN and BLUETOOTH_CONNECT. The AndroidManifest.xml should use android:maxSdkVersion="30" on legacy permissions and android:minSdkVersion="31" on new permissions. The Java code must conditionally request permissions using Build.VERSION.SDK_INT >= Build.VERSION_CODES.S check to avoid requesting non-existent permissions on older Android versions.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src/js/protocols/TauriSerial.js:203-259
Timestamp: 2025-10-25T21:16:32.474Z
Learning: In TauriSerial (src/js/protocols/TauriSerial.js), the requestPermissionDevice() method is not needed and not invoked. Tauri automatically discovers serial devices through the constructor's loadDevices() and startDeviceMonitoring() calls, bypassing the browser permission model that WebSerial requires. Devices are auto-detected via a 1-second polling interval without user permission prompts.

Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4521
File: src/js/protocols/WebSerial.js:148-151
Timestamp: 2025-06-19T22:13:09.136Z
Learning: In WebSerial.js, there's a timing issue where the cached `this.ports` array doesn't immediately reflect newly permitted devices after `requestPermissionDevice()` completes. The `getDevices()` method needs to refresh the device list from the browser API to return accurate data immediately following a permission request and user acceptance.

Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.

@haslinghuis
Copy link
Member Author

Closing in favor of #4735

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

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants