Capacitor BLE plugin using Nordic library #4735
Conversation
WalkthroughAdds native Android BLE support: Nordic BLE libraries and permissions, a new Capacitor Android plugin (BetaflightBlePlugin) implementing scanning/GATT, a JS bridge (CapacitorBle) and protocol registration, minor port-handler selection changes, and plugin registration in MainActivity. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as App/UI
participant JS as CapacitorBle (JS)
participant Plugin as BetaflightBlePlugin (Android)
participant Manager as BleBridgeManager (Nordic)
participant Device as BLE Device
User->>UI: trigger scan / connect / send / disconnect
UI->>JS: getDevices() / requestPermissionDevice() / connect(path) / send(data) / disconnect()
JS->>Plugin: Capacitor bridge call (getDevices/connect/send/disconnect)
Plugin->>Manager: startScan() / connect()/ write()/ disconnect()
Manager->>Device: BLE scanning / GATT connect / write / enable notifications
Device-->>Manager: adverts / notifications / disconnect
Manager-->>Plugin: scan results / connection events / notifications
Plugin-->>JS: callback results / emit events ("connect","receive","disconnect")
JS-->>UI: propagate events with Uint8Array payloads
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
android/app/src/main/AndroidManifest.xml (1)
69-69: Consider making BLE feature optional for broader device compatibility.Setting
android:required="true"forbluetooth_lewill prevent installation on devices without BLE hardware. If the app supports other connection methods (Serial, TCP), consider settingandroid:required="false"to allow installation on non-BLE devices.src/js/protocols/CapacitorBle.js (1)
125-125: Baud rate storage is unnecessary for BLE.BLE connections don't use baud rates (it's a packetized protocol, not a serial stream). Storing
bitratefromoptions?.baudRatemay be misleading. Consider removing this assignment or documenting why it's kept for compatibility.android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java (2)
197-198: Consider canceling pending callbacks when starting fallback scan.When
startFallbackScanis called from line 205, the handler already has a pending callback from line 198. Consider usinghandler.removeCallbacksAndMessages(null)before posting the new delayed callback to prevent multiple finishScan invocations.Apply this approach:
private void startFallbackScan(PluginCall call) { if (scanner == null) { call.reject("Bluetooth LE scanner unavailable"); return; } + handler.removeCallbacksAndMessages(null); fallbackScan = true; scanning = true;Also applies to: 239-240
324-331: Connection enqueue lacks explicit error handling for queue failures.While the
.fail()handler at lines 327-330 covers connection failures, if the.enqueue()call itself throws an exception (e.g., if the BLE queue is full or the adapter is disabled), the PluginCall may never resolve or reject, leaving the JS layer waiting indefinitely.Wrap the enqueue in a try-catch:
- bleManager.connect(device) + try { + bleManager.connect(device) .useAutoConnect(false) .timeout(15_000) .fail((dev, status) -> { connectedAddress = null; call.reject("Connection failed: " + status); }) .enqueue(); + } catch (Exception e) { + connectedAddress = null; + call.reject("Failed to enqueue connection: " + e.getMessage()); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
android/app/build.gradle(1 hunks)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/ble/BetaflightBlePlugin.java(1 hunks)src/js/main.js(1 hunks)src/js/port_handler.js(3 hunks)src/js/protocols/CapacitorBle.js(1 hunks)src/js/serial.js(2 hunks)src/js/utils/checkCompatibility.js(1 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 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-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.
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: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: 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: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-12-10T17:48:24.243Z
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.
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: 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.
📚 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:
android/app/build.gradleandroid/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/AndroidManifest.xmlandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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: 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/main.jssrc/js/serial.jssrc/js/protocols/CapacitorBle.jsandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
📚 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/port_handler.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/port_handler.jssrc/js/serial.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/port_handler.jssrc/js/serial.jssrc/js/protocols/CapacitorBle.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:
src/js/port_handler.jsandroid/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/AndroidManifest.xmlsrc/js/serial.jssrc/js/utils/checkCompatibility.jssrc/js/protocols/CapacitorBle.jsandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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/port_handler.jssrc/js/serial.jssrc/js/protocols/CapacitorBle.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/port_handler.jssrc/js/serial.jssrc/js/protocols/CapacitorBle.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/port_handler.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/port_handler.jsandroid/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/AndroidManifest.xmlsrc/js/serial.jssrc/js/utils/checkCompatibility.jssrc/js/protocols/CapacitorBle.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:
android/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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.javasrc/js/serial.jssrc/js/protocols/CapacitorBle.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:
android/app/src/main/AndroidManifest.xmlsrc/js/utils/checkCompatibility.js
📚 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/CapacitorBle.js
📚 Learning: 2025-12-10T17:48:24.243Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-10T17:48:24.243Z
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.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
🧬 Code graph analysis (2)
src/js/port_handler.js (1)
src/js/serial.js (2)
serial(232-232)serial(232-232)
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java (1)
src/js/protocols/CapacitorBle.js (2)
bytes(11-11)plugin(5-5)
⏰ 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: build
🔇 Additional comments (18)
src/js/utils/checkCompatibility.js (1)
144-145: LGTM!Correctly enables Bluetooth support detection for Android native platform, aligning with the new CapacitorBle protocol implementation.
src/js/port_handler.js (3)
162-193: LGTM!The refactored permission flow correctly:
- Updates the device list first
- Selects the granted port if available
- Falls back to first available device
- Resets to DEFAULT_PORT when no devices exist
The use of the IIFE for
availablePortsis clean and ensures the correct port array is referenced based on protocol type.
210-211: LGTM!Adding BLE-specific display names ("SpeedyBee", "Nordic NRF") to the device filter improves automatic selection of Betaflight-compatible BLE devices.
344-365: Good defensive handling for BLE connection stability.Preserving the existing Bluetooth port list when scans return empty (but a connection or selection is active) prevents losing the selected port mid-connection. This is important because BLE scans can intermittently return empty results while a device remains connected.
src/js/serial.js (2)
7-7: LGTM!Import follows the existing pattern for protocol implementations.
24-29: LGTM!The CapacitorBle protocol is correctly registered for Android, following the established pattern. The "bluetooth" name ensures proper protocol selection via
portPath.startsWith("bluetooth")inselectProtocol().Based on learnings, ensure the CapacitorBle implementation uses idempotent listener attachment (attaching native plugin listeners exactly once per app lifetime) to avoid duplicate handlers and MSP queue pressure.
android/app/build.gradle (1)
39-39: LGTM - Nordic BLE library version 2.11.0 is current.Version 2.11.0 is the latest stable release of the Nordic Semiconductor Android BLE library. The library is well-maintained and widely used for Android BLE development.
src/js/main.js (1)
19-24: LGTM.The defensive checks ensure robustness, and
Capacitor.isLoggingEnabledis a documented property in the official@capacitor/coreAPI. This is a solid developer experience improvement that reduces console noise on native platforms.android/app/src/main/java/betaflight/configurator/MainActivity.java (1)
6-6: LGTM!The BLE plugin registration follows the same pattern as the existing Serial and TCP plugins.
Also applies to: 14-14
android/app/src/main/AndroidManifest.xml (1)
58-65: Version-aware permission structure looks good.The permission declarations correctly follow Android version requirements: legacy BLUETOOTH/BLUETOOTH_ADMIN with
maxSdkVersion="30", and new BLUETOOTH_SCAN/BLUETOOTH_CONNECT for Android 12+. TheneverForLocationflag on BLUETOOTH_SCAN properly indicates that location is not the purpose of scanning.Note: ACCESS_COARSE_LOCATION (line 60) is required for Android 8-11 for BLE scanning, and the Java code at lines 111-120 of BetaflightBlePlugin.java properly checks permissions based on Android version, which aligns with the learnings on version-aware permission handling.
src/js/protocols/CapacitorBle.js (3)
27-54: Event listener setup follows the singleton pattern correctly.The listeners are attached in the constructor, which is appropriate since protocol implementations are instantiated once and never destroyed. This matches the learning that native plugin listeners should be attached exactly once per app lifetime.
166-186: LGTM!The send method properly handles the disconnected state, base64 encoding, error handling, and callback invocation.
70-70: Path naming conventionbluetooth-${d.address}is intentional and consistent with the codebase.The address-specific naming (using Bluetooth MAC addresses) is the correct approach for this implementation. It provides unique, persistent device identification across connections, similar to how
webusbdfu.jsusesusb_${serialNumber}andCapacitorSerial.jsusescapacitor-${deviceId}. The path is properly used throughout the class for device lookup and connection management.android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java (5)
107-121: LGTM! Version-aware permission checking implemented correctly.The permission checking properly differentiates between Android 12+ (API 31+) requiring BLUETOOTH_SCAN and BLUETOOTH_CONNECT, versus Android 8-11 requiring BLUETOOTH, BLUETOOTH_ADMIN, and ACCESS_COARSE_LOCATION. This aligns with the learnings on version-aware BLE permission handling.
Based on learnings about Android BLE permission requirements across API levels.
281-322: Connection observer setup is comprehensive.The ConnectionObserver properly handles all lifecycle states (connecting, connected, failed, ready, disconnecting, disconnected) and correctly resolves/rejects the PluginCall based on the outcome. The
onDeviceReadycallback (lines 303-307) is the appropriate place to resolve the connection, as it signals that services are discovered and notifications are enabled.This aligns with the learning that the connection should only be announced after GATT services are resolved and notifications are started.
584-595: LGTM! Write characteristic caching follows best practices.The write characteristic is resolved and cached in
isRequiredServiceSupported()(line 584), the write type is determined and set based on PROPERTY_WRITE_NO_RESPONSE (lines 591-593), and the cached characteristic is used in thesend()method (line 616). Characteristics are properly cleared inonServicesInvalidated()(lines 607-608).Based on learnings about caching write characteristics once in onServicesDiscovered and using cached values in write operations.
612-617: Send method correctly uses cached write characteristic.The
send()method is strictly "write-only" - it uses the cachedwriteCharacteristicwithout performing any UUID lookups or characteristic resolution. The.split()call enables packet splitting for large payloads, which is appropriate for BLE's MTU constraints.This matches the learning that write methods should be "write-only" and use cached characteristics.
143-199: The timeout coordination is safe—callbacks execute sequentially, not concurrently.When the primary scan timeout fires at 5 seconds,
finishScan()executes, callsstopScan(), and if no devices are found, immediately returns after posting the fallback timeout. The fallback timeout is not posted until the primary timeout has already fired and completed. SubsequentfinishScan()callbacks fire at 9 seconds (5s + 4s), after the primary one has fully executed. There is no race condition with multiple pending callbacks.
|
Preview URL: https://pr4735.betaflight-app-preview.pages.dev |
haslinghuis
force-pushed
the
capacitor-ble-plugin
branch
from
e74da72 to
6de4e92
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/js/port_handler.js (1)
162-174:updatedPortsvariable is unused in practice.The
updatedPortsreturn value is only referenced in thedefaultcase (line 172), which handles unknown protocols that wouldn't have valid ports anyway. SinceupdateDeviceListalready populatesthis.current*Portsarrays before the IIFE executes, the variable serves no practical purpose.Consider simplifying:
- const updatedPorts = await this.updateDeviceList(protocol); - const availablePorts = (() => { - switch (protocol) { - case "bluetooth": - return this.currentBluetoothPorts; - case "usb": - return this.currentUsbPorts; - case "serial": - return this.currentSerialPorts; - default: - return updatedPorts ?? []; - } - })(); + await this.updateDeviceList(protocol); + const availablePorts = + protocol === "bluetooth" ? this.currentBluetoothPorts : + protocol === "usb" ? this.currentUsbPorts : + protocol === "serial" ? this.currentSerialPorts : [];src/js/protocols/CapacitorBle.js (1)
8-8: Add braces to single-line if statements.SonarCloud flags missing braces on single-line
ifstatements. While functionally correct, adding braces improves consistency and prevents errors during future modifications.Apply this diff to add braces:
const base64ToUint8Array = (b64) => { - if (!b64) return new Uint8Array(0); + if (!b64) { + return new Uint8Array(0); + } const binary = atob(b64);async getDevices() { - if (!plugin) return []; + if (!plugin) { + return []; + }async connect(path, options) { - if (!plugin) return false; + if (!plugin) { + return false; + }async disconnect() { - if (!plugin) return false; - if (!this.connected) return true; + if (!plugin) { + return false; + } + if (!this.connected) { + return true; + }shouldBypassCrc(expectedChecksum) { const deviceDescription = this.deviceDescription; - if (!deviceDescription) return false; + if (!deviceDescription) { + return false; + }Also applies to: 61-61, 96-96, 139-140, 160-160
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java (2)
182-186: Consider logging empty service UUIDs for debugging.When
requestedServicesis empty (either from no input or all invalid UUIDs), the code silently falls back toKNOWN_DEVICES. Consider logging this fallback to help diagnose issues where specific service UUIDs are expected but not received.Apply this diff to add debug logging:
if (requestedServices.isEmpty()) { + Log.d(TAG, "No valid service UUIDs requested, using all known devices"); for (String service : KNOWN_DEVICES.keySet()) { requestedServices.add(UUID.fromString(service)); } }
68-88: Document the fallback device name aliases.Lines 86-87 add name-based fallback aliases for SpeedyBee devices. Consider adding a comment explaining that these aliases are used during fallback scans when devices don't advertise service UUIDs, to help future maintainers understand this mapping.
Apply this diff to add clarifying comments:
addDevice("DroneBridge", "0000db32-0000-1000-8000-00805f9b34fb", "0000db33-0000-1000-8000-00805f9b34fb", "0000db34-0000-1000-8000-00805f9b34fb"); - // Extra name aliases that map to known profiles (helps when service UUIDs aren't advertised) + // Fallback name aliases for devices that don't always advertise service UUIDs + // Used during unfiltered scan when service-filtered scan returns no results KNOWN_DEVICES_BY_NAME.put("speedybee f7v3", KNOWN_DEVICES.get("0000abf0-0000-1000-8000-00805f9b34fb")); KNOWN_DEVICES_BY_NAME.put("speedybee", KNOWN_DEVICES.get("0000abf0-0000-1000-8000-00805f9b34fb"));
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
android/app/build.gradle(1 hunks)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/ble/BetaflightBlePlugin.java(1 hunks)src/js/main.js(1 hunks)src/js/port_handler.js(3 hunks)src/js/protocols/CapacitorBle.js(1 hunks)src/js/serial.js(2 hunks)src/js/utils/checkCompatibility.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/js/main.js
- src/js/serial.js
- android/app/src/main/AndroidManifest.xml
- android/app/build.gradle
- src/js/utils/checkCompatibility.js
🧰 Additional context used
🧠 Learnings (15)
📓 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-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.
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: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-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:
android/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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.javaandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.javasrc/js/protocols/CapacitorBle.jssrc/js/port_handler.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/MainActivity.javaandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.javasrc/js/protocols/CapacitorBle.jssrc/js/port_handler.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:
android/app/src/main/java/betaflight/configurator/MainActivity.javasrc/js/protocols/CapacitorBle.js
📚 Learning: 2025-12-10T17:48:24.243Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-10T17:48:24.243Z
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.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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: 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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.javasrc/js/protocols/CapacitorBle.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/CapacitorBle.jssrc/js/port_handler.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/CapacitorBle.jssrc/js/port_handler.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/CapacitorBle.jssrc/js/port_handler.js
📚 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/CapacitorBle.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/port_handler.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/port_handler.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/port_handler.js
🧬 Code graph analysis (1)
src/js/port_handler.js (1)
src/js/serial.js (2)
serial(232-232)serial(232-232)
🪛 GitHub Check: SonarCloud Code Analysis
src/js/protocols/CapacitorBle.js
[failure] 61-61: Expected { after 'if' condition.
[failure] 160-160: Expected { after 'if' condition.
[warning] 13-13: Prefer String#codePointAt() over String#charCodeAt().
[failure] 96-96: Expected { after 'if' condition.
[failure] 139-139: Expected { after 'if' condition.
[failure] 8-8: Expected { after 'if' condition.
[failure] 140-140: Expected { after 'if' condition.
[warning] 21-21: Prefer String.fromCodePoint() over String.fromCharCode().
⏰ 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: build
🔇 Additional comments (10)
src/js/port_handler.js (3)
176-193: Clean permission flow with proper fallback handling.The early return pattern and fallback chain (explicit port → first available → default) handles edge cases well. The flow correctly resets to
DEFAULT_PORTwhen no devices are found after permission request.
211-211: Good addition of BLE device identifiers.Adding "SpeedyBee" and "Nordic NRF" to the device filter enables automatic port selection for common BLE devices, which aligns with the Capacitor BLE plugin feature.
344-365: Good defensive logic to preserve Bluetooth ports during empty scans.This correctly addresses the scenario where BLE scans return empty while an active connection or user selection exists. The condition properly checks for both connected state and selected port prefix, preventing loss of the active Bluetooth port from the UI.
Based on learnings, this pattern is important for maintaining stable Bluetooth connections in the configurator.
src/js/protocols/CapacitorBle.js (3)
43-53: LGTM! Event listeners attached correctly.The constructor attaches native plugin listeners exactly once, which is correct since protocol implementations are singletons in this codebase. The event names "dataReceived" and "disconnected" match the native plugin's
notifyListenerscalls.Based on learnings, this pattern prevents duplicate handlers and MSP queue pressure issues.
7-16: Base64 encoding is correct; ignore SonarCloud warning.SonarCloud suggests using
codePointAt()instead ofcharCodeAt(), but this is a false positive. Base64 encoding requires Latin-1 byte-to-character mapping (0-255 range), andcharCodeAt()is the correct choice here. UsingcodePointAt()would break the encoding for non-ASCII bytes.
95-136: Verify connection timing aligns with GATT readiness.The
connectmethod emits the "connect" event immediately afterplugin.connectresolves. Based on learnings, this should only happen after GATT services are resolved and notifications are started to prevent "Unable to write characteristic" errors.Reviewing the native code (BetaflightBlePlugin.java lines 303-307), the plugin resolves the call in
onDeviceReady, which is called after service discovery and notification setup complete. This timing is correct.Verify that the first MSP write after connection succeeds consistently. If you observe "Unable to write characteristic" errors, it may indicate the native plugin is resolving too early.
Based on learnings about connection timing and GATT service readiness.
android/app/src/main/java/betaflight/configurator/MainActivity.java (1)
6-6: LGTM!The BLE plugin registration follows the same pattern as existing plugins (Serial, TCP). The import and registration are correct.
Also applies to: 14-14
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java (3)
107-121: LGTM! Version-aware permission handling.The permission check correctly handles Android version differences:
- Android 12+ (API 31+): Checks
BLUETOOTH_SCANandBLUETOOTH_CONNECT- Android 8-11 (API 26-30): Checks
BLUETOOTH,BLUETOOTH_ADMIN, andACCESS_COARSE_LOCATIONThis implementation matches the learnings about version-aware permission handling and prevents requesting non-existent permissions on older Android versions.
Based on learnings about Android Bluetooth permission handling across API levels.
576-609: LGTM! Characteristic caching follows best practices.The
ManagerGattCallbackcorrectly implements the characteristic caching pattern:
- Caches
writeCharacteristicandnotifyCharacteristicinisRequiredServiceSupported- Sets write type to
WRITE_TYPE_NO_RESPONSEwhen supported (preferred for throughput)- Enables notifications in
initialize()- Clears cached characteristics in
onServicesInvalidated()This follows the learnings about caching characteristics once during service discovery and clearing them on disconnect.
Based on learnings about write-only pattern and characteristic caching.
303-307: LGTM! Connection resolves at the correct time.The
connectcall is resolved inonDeviceReady, which is called after service discovery and notification setup are complete. This ensures the JavaScript layer only receives the "connect" event after the GATT connection is fully ready for data transfer, preventing "Unable to write characteristic" errors.Based on learnings about connection timing and GATT service readiness.
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
Show resolved
Hide resolved
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java (1)
603-606: LGTM: MTU negotiation properly implemented.The code correctly requests MTU (247 bytes) during connection initialization and stores the negotiated value for use in data transmission. This addresses the concern from previous reviews about ensuring efficient BLE writes by negotiating a larger MTU than the default 23 bytes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java(1 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 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-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.
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: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-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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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/protocols/ble/BetaflightBlePlugin.java
📚 Learning: 2025-12-10T17:48:24.243Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-10T17:48:24.243Z
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.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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: 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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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/protocols/ble/BetaflightBlePlugin.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/ble/BetaflightBlePlugin.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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
📚 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/ble/BetaflightBlePlugin.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/ble/BetaflightBlePlugin.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/ble/BetaflightBlePlugin.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/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
🪛 GitHub Actions: Deployment (PR and Push)
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
[error] 625-625: no suitable method found for split(int). The call to split(chunkSize) uses an int, but WriteRequest.split(...) requires DataSplitter or WriteProgressCallback.
🔇 Additional comments (9)
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java (9)
47-59: LGTM: Plugin permissions properly declared.The permission annotation correctly includes all necessary BLE permissions (BLUETOOTH_SCAN, BLUETOOTH_CONNECT, ACCESS_COARSE_LOCATION) under a single "bluetooth" alias, which aligns with the version-aware permission handling implemented in the permission check methods.
65-88: LGTM: Known device registry enables flexible discovery.The dual-map approach (by service UUID and by name) allows matching devices both by advertised service UUIDs and by device names during fallback scans, which is essential for devices that don't advertise service UUIDs properly.
109-142: LGTM: Version-aware permission handling implemented correctly.The implementation properly checks different permissions based on Android version (BLUETOOTH_SCAN/BLUETOOTH_CONNECT for API 31+, BLUETOOTH/BLUETOOTH_ADMIN/ACCESS_COARSE_LOCATION for earlier versions), which aligns with Android's BLE permission model changes.
Based on learnings, this matches the required pattern for Android BLE permission handling.
144-243: LGTM: Two-phase scanning strategy handles devices with and without advertised service UUIDs.The implementation uses filtered scanning (by service UUID) as the primary method, falling back to unfiltered scanning with name-based device matching if no results are found. This balances power efficiency with device discoverability, which is appropriate for native Android BLE (unlike Web Bluetooth's
acceptAllDevicespattern).
250-334: LGTM: Connection lifecycle properly managed with Nordic BLE library.The implementation creates a new
BleBridgeManagerinstance per connection, sets up connection observers to emit events to the JS layer, and uses Nordic BLE library's fluent API with appropriate timeout (15s) and error handling.
360-389: LGTM: Send method follows write-only pattern.The implementation correctly delegates to the pre-configured
bleManager.send()without performing per-call UUID parsing or characteristic lookups, and accurately reports the original payload length asbytesSent(the internal fragmentation is transparent to the caller).Based on learnings, the write-only pattern is preferred for the public send method.
391-402: LGTM: Cleanup properly stops scans and closes BLE manager.The
handleOnDestroy()method ensures that active scans are stopped and the BLE manager is closed when the plugin is destroyed, preventing resource leaks.
581-599: LGTM: Characteristics cached and write type determined once during service discovery.The implementation correctly caches
writeCharacteristicandnotifyCharacteristicreferences inisRequiredServiceSupported(), sets up notification callbacks, and configures the write type based on characteristic properties. This matches the pattern from learnings where characteristic resolution happens once during service discovery rather than per-write operation.Based on learnings, caching characteristics and write type in onServicesDiscovered (isRequiredServiceSupported) is the preferred pattern.
517-525: LGTM: Notification handler correctly bridges native BLE data to JS layer.The method receives binary data from BLE notifications, encodes it as base64, and emits it to JavaScript listeners via the "dataReceived" event, which is the appropriate pattern for binary data transfer between native and JavaScript.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java (1)
620-626: Remove the problematic split() call and unused chunkSize calculation.As identified in the previous review, line 625 is missing the problematic
.split(chunkSize)call that causes a compilation error becauseWriteRequest.split()does not accept anintparameter. The Nordic BLE library automatically handles fragmentation based on the negotiated MTU whenWRITE_TYPE_NO_RESPONSEis set (line 595).Additionally, the
chunkSizecalculation at line 624 is unused and should be removed for clarity.The current code at line 625 is:
return writeCharacteristic(writeCharacteristic, data);If the problematic
split(chunkSize)call is present, apply this diff:WriteRequest send(byte[] data) { if (writeCharacteristic == null) { return null; } - int chunkSize = Math.max(negotiatedMtu - 3, 20); // 3 bytes ATT overhead - return writeCharacteristic(writeCharacteristic, data).split(chunkSize); + return writeCharacteristic(writeCharacteristic, data); }The library will automatically split packets based on the
negotiatedMtuwhen usingWRITE_TYPE_NO_RESPONSE.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java(1 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 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-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.
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: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-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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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/protocols/ble/BetaflightBlePlugin.java
📚 Learning: 2025-12-10T17:48:24.243Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-10T17:48:24.243Z
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.
Applied to files:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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: 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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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/protocols/ble/BetaflightBlePlugin.java
📚 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/ble/BetaflightBlePlugin.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/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
📚 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/ble/BetaflightBlePlugin.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/ble/BetaflightBlePlugin.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/ble/BetaflightBlePlugin.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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
⏰ 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 (3)
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java (3)
144-243: LGTM! Well-designed two-phase scanning approach.The scanning logic correctly implements a two-phase approach: first scanning with service UUID filters (lines 190-200), then falling back to an unfiltered scan with name matching (lines 228-242) if no devices are found. This balances efficiency with device discovery completeness. The exception handling for malformed service UUID arrays (lines 169-181) and permission/adapter checks are appropriate.
391-452: LGTM! Proper lifecycle management and cleanup.The lifecycle management is well-implemented with appropriate cleanup in
handleOnDestroy()(lines 391-402) and exception handling for both BLE manager closure (line 398) and scan stopping (line 409). The dual scan callback structure correctly delegates tohandleResult().
454-515: LGTM! Robust profile matching logic.The result handling correctly prioritizes service UUID matching over name-based matching, updates RSSI for duplicate discoveries (line 468), and implements a fallback hierarchy that aligns with the two-phase scan strategy.
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
Show resolved
Hide resolved
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
Show resolved
Hide resolved
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
Show resolved
Hide resolved
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
Show resolved
Hide resolved
haslinghuis
force-pushed
the
capacitor-ble-plugin
branch
from
9de39f1 to
230f144
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java (3)
145-153: Fix permission callback to resume the correct originating method.The
onBlePermissionResultcallback always invokesgetDevices(call), but permissions can be requested from bothgetDevices()(line 157) andconnect()(line 263). When a user callsconnect()without permissions, the callback incorrectly resumesgetDevices()instead ofconnect(), breaking the connection flow.Store the originating method name when requesting permissions:
+ private String pendingCallMethod = null; + @PermissionCallback private void onBlePermissionResult(PluginCall call) { if (hasBlePermissions()) { - // Continue the original request now that permission is granted - getDevices(call); + if ("connect".equals(pendingCallMethod)) { + connect(call); + } else { + getDevices(call); + } + pendingCallMethod = null; } else { call.reject("Bluetooth permission denied"); + pendingCallMethod = null; } }Update
ensurePermissions()to save the method name:- private boolean ensurePermissions(PluginCall call) { + private boolean ensurePermissions(PluginCall call, String methodName) { if (hasBlePermissions()) { return true; } + pendingCallMethod = methodName; if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { requestPermissionForAlias("bluetooth", call, "onBlePermissionResult"); } else { requestPermissionForAlias("bluetoothLegacy", call, "onBlePermissionResult"); } return false; }Update callers to pass method names:
- if (!ensurePermissions(call)) { + if (!ensurePermissions(call, "getDevices")) { return; }- if (!ensurePermissions(call)) { + if (!ensurePermissions(call, "connect")) { return; }
294-344: Prevent duplicate call resolution in connection callbacks.Multiple callbacks can resolve or reject the same
PluginCall:onDeviceFailedToConnect()(line 310),onDeviceReady()(line 316), andconnect().fail()(line 340). IfonDeviceFailedToConnect()fires and thenconnect().fail()also triggers, a duplicate rejection occurs, causing a runtime exception.Guard against duplicate resolution with an atomic flag:
+ final AtomicBoolean callResolved = new AtomicBoolean(false); + bleManager.setConnectionObserver(new ConnectionObserver() { @Override public void onDeviceConnecting(@NonNull BluetoothDevice device) { Log.d(TAG, "Connecting to " + device.getAddress()); } @Override public void onDeviceConnected(@NonNull BluetoothDevice device) { Log.d(TAG, "Connected to " + device.getAddress()); connectedAddress = device.getAddress(); JSObject evt = new JSObject(); evt.put("address", connectedAddress); notifyListeners("connected", evt); } @Override public void onDeviceFailedToConnect(@NonNull BluetoothDevice device, int reason) { connectedAddress = null; + if (callResolved.compareAndSet(false, true)) { call.reject("Connection failed: " + reason); + } } @Override public void onDeviceReady(@NonNull BluetoothDevice device) { + if (callResolved.compareAndSet(false, true)) { JSObject res = new JSObject(); res.put("success", true); call.resolve(res); + } } @Override public void onDeviceDisconnecting(@NonNull BluetoothDevice device) { Log.d(TAG, "Disconnecting " + device.getAddress()); } @Override public void onDeviceDisconnected(@NonNull BluetoothDevice device, int reason) { connectedAddress = null; JSObject evt = new JSObject(); evt.put("address", device.getAddress()); evt.put("reason", reason); notifyListeners("disconnected", evt); } }); bleManager.connect(device) .useAutoConnect(false) .timeout(15_000) .fail((dev, status) -> { connectedAddress = null; + if (callResolved.compareAndSet(false, true)) { call.reject("Connection failed: " + status); + } }) .enqueue();Import
java.util.concurrent.atomic.AtomicBooleanat the top of the file.
371-400: Add error handling for Base64 decoding.The
Base64.decode()call at line 384 can throwIllegalArgumentExceptionif the input is malformed. While invalid input is a client error, gracefully rejecting the call improves robustness.Apply this diff:
@PluginMethod public void send(PluginCall call) { if (bleManager == null || !bleManager.isConnected()) { call.reject("Not connected"); return; } String b64 = call.getString("data"); if (b64 == null || b64.isEmpty()) { call.reject("data is required"); return; } - byte[] payload = Base64.decode(b64, Base64.NO_WRAP); + byte[] payload; + try { + payload = Base64.decode(b64, Base64.NO_WRAP); + } catch (IllegalArgumentException e) { + call.reject("Invalid base64 data: " + e.getMessage()); + return; + } + WriteRequest request = bleManager.send(payload); if (request == null) { call.reject("Not ready to send data"); return; } request .done(device -> { JSObject res = new JSObject(); res.put("bytesSent", payload.length); call.resolve(res); }) .fail((device, status) -> call.reject("Send failed: " + status)) .enqueue(); }
🧹 Nitpick comments (2)
src/js/protocols/CapacitorBle.js (1)
43-53: Consider adding idempotent listener guard.The native plugin listeners are attached in the constructor without a guard to prevent duplicate registration. While protocol implementations are singletons, explicitly guarding against re-attachment (e.g., with a static flag or
_attachNativeListenersOnce()) would make this more robust.Based on learnings, duplicate listener attachment in similar BLE implementations can lead to duplicate handlers and increased MSP queue pressure.
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java (1)
631-637: Remove unused chunkSize calculation.The
chunkSizevariable is calculated at line 635 but never used. The Nordic BLE library automatically handles payload fragmentation based on the negotiated MTU when usingWRITE_TYPE_NO_RESPONSE.Apply this diff:
WriteRequest send(byte[] data) { if (writeCharacteristic == null) { return null; } - int chunkSize = Math.max(negotiatedMtu - 3, 20); // 3 bytes ATT overhead return writeCharacteristic(writeCharacteristic, data); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
android/app/build.gradle(1 hunks)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/ble/BetaflightBlePlugin.java(1 hunks)src/js/main.js(1 hunks)src/js/port_handler.js(3 hunks)src/js/protocols/CapacitorBle.js(1 hunks)src/js/serial.js(2 hunks)src/js/utils/checkCompatibility.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- android/app/build.gradle
- src/js/utils/checkCompatibility.js
- src/js/main.js
- src/js/serial.js
🧰 Additional context used
🧠 Learnings (24)
📓 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-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.
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: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-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/port_handler.jsandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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/port_handler.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/port_handler.jsandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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:
src/js/port_handler.jssrc/js/protocols/CapacitorBle.jsandroid/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/AndroidManifest.xmlandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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/port_handler.jssrc/js/protocols/CapacitorBle.js
📚 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/port_handler.jsandroid/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/AndroidManifest.xmlandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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/port_handler.jssrc/js/protocols/CapacitorBle.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/port_handler.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/port_handler.jssrc/js/protocols/CapacitorBle.jsandroid/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/AndroidManifest.xmlandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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: 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/CapacitorBle.jsandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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/CapacitorBle.jsandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
📚 Learning: 2025-12-10T17:48:24.243Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-10T17:48:24.243Z
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.
Applied to files:
android/app/src/main/java/betaflight/configurator/MainActivity.javaandroid/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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-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
📚 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/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
📚 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/ble/BetaflightBlePlugin.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/ble/BetaflightBlePlugin.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/ble/BetaflightBlePlugin.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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.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/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java
🧬 Code graph analysis (1)
src/js/port_handler.js (1)
src/js/serial.js (2)
serial(232-232)serial(232-232)
🔇 Additional comments (5)
android/app/src/main/java/betaflight/configurator/MainActivity.java (1)
6-6: LGTM!The BLE plugin registration follows the established pattern for serial and TCP plugins.
Also applies to: 14-14
android/app/src/main/AndroidManifest.xml (1)
58-71: LGTM!The BLE permissions and feature declarations correctly follow version-aware patterns: legacy permissions (BLUETOOTH, BLUETOOTH_ADMIN, ACCESS_COARSE_LOCATION) are scoped to API 26-30, while modern permissions (BLUETOOTH_SCAN, BLUETOOTH_CONNECT) target API 31+.
Based on learnings, Android BLE permission handling must be version-aware to avoid requesting non-existent permissions on older versions.
src/js/port_handler.js (2)
211-211: LGTM!Expanding the device filter to include "SpeedyBee" and "Nordic NRF" correctly supports BLE device auto-selection alongside existing serial identifiers.
344-365: LGTM!Preserving the Bluetooth port list when a scan returns empty but a connection or selection exists prevents losing the active port. This defensive logic correctly handles the case where scanning doesn't always detect already-connected devices.
android/app/src/main/java/betaflight/configurator/protocols/ble/BetaflightBlePlugin.java (1)
613-622: LGTM!The initialization sequence correctly requests MTU negotiation before enabling notifications, and the Nordic BLE library ensures
onDeviceReady()is called only afterinitialize()completes. This guarantees services are discovered and notifications are active before the JS layer receives the connect event.Based on learnings, announcing connection before service discovery causes MSP write failures.
haslinghuis
force-pushed
the
capacitor-ble-plugin
branch
from
230f144 to
4438adc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java (4)
175-183: Fix permission callback to resume the correct original method (still unresolved).The permission callback still always calls
getDevices(call), but permission requests can originate from eithergetDevices()(line 187) orconnect()(line 295). Whenconnect()requests permissions, the callback incorrectly resumesgetDevices()instead, breaking the connection flow.This issue was previously flagged and remains unaddressed. Please track the originating method and resume the correct flow in
onBlePermissionResult.
326-376: Prevent duplicate call resolution in connection callbacks (still unresolved).Multiple callbacks can still resolve or reject the same
PluginCall:
onDeviceFailedToConnect()at line 344onDeviceReady()at line 351connect().fail()at line 374If a connection fails, both
onDeviceFailedToConnectandconnect().fail()may fire, causing a duplicate rejection and potential runtime exception.This issue was previously flagged and remains unaddressed. Please add a guard (e.g.,
AtomicBooleanor synchronized flag) to ensure the call is resolved/rejected only once.
403-432: Add error handling for Base64 decoding (still unresolved).Line 416's
Base64.decode()can throwIllegalArgumentExceptionif the input is malformed, causing an unhandled exception. While this is a client error, gracefully rejecting the call improves robustness.This issue was previously flagged and remains unaddressed. Please wrap the decode in a try-catch and reject the call with a clear error message.
434-445: Remove pending handler callbacks in handleOnDestroy (still unresolved).Handler callbacks scheduled at lines 234 and 281 are not removed when the plugin is destroyed. If
handleOnDestroy()is called while a scan is in progress, the pending callback will still fire after cleanup, potentially callingfinishScan()on invalid state or attempting to resolve a stalePluginCall.This issue was previously flagged and remains unaddressed. Add
handler.removeCallbacksAndMessages(null);at the beginning ofhandleOnDestroy()to cancel all pending callbacks before cleanup.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java(1 hunks)
🧰 Additional context used
🧠 Learnings (16)
📓 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: 0
File: :0-0
Timestamp: 2025-12-14T20:22:03.568Z
Learning: In betaflight-configurator's msp.js, the bluetooth CRC bypass check at line 266 uses serial.protocol === "bluetooth". When protocol names are unified to "ble", this check must be updated to serial.protocol === "ble".
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-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: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: 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().
📚 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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
📚 Learning: 2025-12-10T17:48:24.243Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-10T17:48:24.243Z
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.
Applied to files:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.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/app/protocols/ble/BetaflightBlePlugin.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/app/protocols/ble/BetaflightBlePlugin.java
📚 Learning: 2025-12-14T20:22:03.568Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-14T20:22:03.568Z
Learning: In betaflight-configurator's msp.js, the bluetooth CRC bypass check at line 266 uses serial.protocol === "bluetooth". When protocol names are unified to "ble", this check must be updated to serial.protocol === "ble".
Applied to files:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
📚 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/app/protocols/ble/BetaflightBlePlugin.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/app/protocols/ble/BetaflightBlePlugin.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/app/protocols/ble/BetaflightBlePlugin.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: 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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.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/app/protocols/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
⏰ 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 (1)
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java (1)
721-729: MTU negotiation implemented correctly.The implementation properly requests MTU negotiation at line 722 with the desired MTU of 247, which significantly improves BLE throughput by reducing packet fragmentation. This addresses the concern raised in previous discussions about the 64-byte limitation.
As noted in the earlier investigation, while typical MSP messages are small, MTU negotiation still provides substantial benefits:
- Default MTU 23 = ~17 bytes per packet (after ATT overhead)
- Negotiated MTU 247 = ~244 bytes per packet
- A 60-byte message: 4 packets → 1 packet
- A 200-byte dataflash read: 12+ packets → 1 packet
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java (4)
175-183: Permission callback routing issue still present.The
onBlePermissionResultcallback always invokesgetDevices(call)(line 179), but permission requests can originate from eithergetDevices()orconnect()(line 295). When a user callsconnect()without permissions and then grants them, the callback incorrectly invokesgetDevices()instead of resumingconnect(), breaking the connection flow.This issue was flagged in a previous review and remains unresolved.
Based on learnings, permission handling must properly route callbacks to the originating method.
326-376: Duplicate call resolution issue still present.Multiple code paths can resolve or reject the same
PluginCall:onDeviceFailedToConnect()(line 344),onDeviceReady()(line 351), andconnect().fail()(line 374). IfonDeviceFailedToConnect()fires, thenconnect().fail()may also trigger, causing a duplicate rejection and potential runtime exception.This issue was flagged in a previous review and remains unresolved. An atomic guard (e.g.,
AtomicBoolean) should test-and-set before anycall.resolve()orcall.reject()to prevent double-resolution.
403-432: Base64 decoding error handling still missing.The
Base64.decode()call at line 416 can throwIllegalArgumentExceptionif the input is malformed, causing an unhandled exception. While invalid input indicates a client error, gracefully rejecting the call improves robustness.This issue was flagged in a previous review and remains unresolved.
Apply this diff to add error handling:
String b64 = call.getString("data"); if (b64 == null || b64.isEmpty()) { call.reject("data is required"); return; } - byte[] payload = Base64.decode(b64, Base64.NO_WRAP); + byte[] payload; + try { + payload = Base64.decode(b64, Base64.NO_WRAP); + } catch (IllegalArgumentException e) { + call.reject("Invalid base64 data: " + e.getMessage()); + return; + } + WriteRequest request = bleManager.send(payload);
434-457: Handler callbacks not removed during destroy.Handler callbacks scheduled at lines 234 and 281 are not cancelled when the plugin is destroyed. If
handleOnDestroy()is called while a scan is in progress, the pending callbacks will still fire after cleanup, potentially callingfinishScan()on invalid state or attempting to resolve a stalePluginCall.This issue was flagged in a previous review and remains unresolved.
Apply this diff to remove pending callbacks:
@Override protected void handleOnDestroy() { + handler.removeCallbacksAndMessages(null); stopScan(); try {
🧹 Nitpick comments (3)
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java (3)
185-239: Consider guarding against concurrent scan requests.If
getDevices()is called while a scan is already in progress, the method clearsdiscoveredDevices(line 200) and starts a new scan without canceling the previous handler callback (line 234). This can lead to multiplefinishScan()invocations with overlapping state, potentially causing confusing results.Consider checking the
scanningflag at the method entry and either rejecting concurrent requests or explicitly stopping the previous scan first.
379-401: disconnect() doesn't handle connecting state.Lines 381-386 check if
bleManageris null or not connected and immediately resolve with success. However, ifdisconnect()is called while the device is still connecting (not yet fully connected), this check passes and the method returns without actually stopping the connection attempt, potentially leaving the connection in progress.Consider calling
bleManager.disconnect()orbleManager.close()regardless of connection state whenbleManageris non-null, to ensure proper cleanup.
720-725: Consider removing unused negotiatedMtu variable.The
negotiatedMtuvariable is stored (line 723) but never used in subsequent operations. While the MTU negotiation itself is still effective (the Nordic library automatically uses the negotiated MTU for writes), the unused variable could be removed or documented.If you plan to use it for diagnostics or logging, consider adding a comment explaining its purpose.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java(1 hunks)
🧰 Additional context used
🧠 Learnings (17)
📓 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: 0
File: :0-0
Timestamp: 2025-12-14T20:22:03.568Z
Learning: In betaflight-configurator's msp.js, the bluetooth CRC bypass check at line 266 uses serial.protocol === "bluetooth". When protocol names are unified to "ble", this check must be updated to serial.protocol === "ble".
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-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: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: 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().
📚 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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
📚 Learning: 2025-12-10T17:48:24.243Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-10T17:48:24.243Z
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.
Applied to files:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.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/app/protocols/ble/BetaflightBlePlugin.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: 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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.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/app/protocols/ble/BetaflightBlePlugin.java
📚 Learning: 2025-12-14T20:22:03.568Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-12-14T20:22:03.568Z
Learning: In betaflight-configurator's msp.js, the bluetooth CRC bypass check at line 266 uses serial.protocol === "bluetooth". When protocol names are unified to "ble", this check must be updated to serial.protocol === "ble".
Applied to files:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
📚 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/app/protocols/ble/BetaflightBlePlugin.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/app/protocols/ble/BetaflightBlePlugin.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/app/protocols/ble/BetaflightBlePlugin.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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.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/app/protocols/ble/BetaflightBlePlugin.java
📚 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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.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:
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
📚 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/app/protocols/ble/BetaflightBlePlugin.java
⏰ 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: Build / build
🔇 Additional comments (3)
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java (3)
1-144: Well-structured plugin implementation with Nordic library integration.The plugin structure is clean and well-organized:
- Constants are grouped logically by BLE profile
- Permission annotations follow Android version requirements
- Nordic BLE library integration provides robust GATT handling
- Handler on main looper ensures thread-safe operation
- MTU negotiation target of 247 is appropriate for BLE throughput
459-608: Scan result handling is well-designed.The two-stage discovery approach is effective:
- Primary scan matches by advertised service UUIDs (lines 531-549)
- Fallback scan additionally matches by device name (lines 552-563)
- Unknown devices are logged once per address with detailed advertisement data (lines 568-608)
This strategy balances fast discovery for standard devices with compatibility for devices that don't advertise service UUIDs correctly.
648-745: BleBridgeManager implementation follows best practices.The Nordic BLE library integration is well-implemented:
- Write characteristic cached in
isRequiredServiceSupported()per learnings- MTU negotiation (line 722) enables optimal throughput
- Characteristics properly cleared in
onServicesInvalidated()(lines 733-736)- SpeedyBee fallback profile switching (lines 675-704) provides excellent device compatibility
- Nordic library's
writeCharacteristic()automatically handles data splitting based on negotiated MTUBased on learnings, write characteristics should be cached once and the Nordic library handles MTU-aware data splitting.
android/app/src/main/java/betaflight/app/protocols/ble/BetaflightBlePlugin.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/js/port_handler.js (2)
162-174: Approve improved device list refresh with optional simplification.The logic correctly refreshes the device list after permission and computes available ports. However, since
updateDeviceList(protocol)both updates the correspondingcurrent*Portsarray and returns the same data, the switch statement to computeavailablePortsis functionally redundant—you could simplify by usingupdatedPortsdirectly.Optional simplification:
const updatedPorts = await this.updateDeviceList(protocol); -const availablePorts = (() => { - switch (protocol) { - case "bluetooth": - return this.currentBluetoothPorts; - case "usb": - return this.currentUsbPorts; - case "serial": - return this.currentSerialPorts; - default: - return updatedPorts ?? []; - } -})(); +const availablePorts = updatedPorts ?? [];
344-349: Minor style inconsistency with other cases.The bluetooth case now has braces while the usb and serial cases don't, creating a minor style inconsistency. Consider either removing the braces here or adding them to the other cases for uniformity.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/js/port_handler.js(2 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 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: 0
File: :0-0
Timestamp: 2025-12-14T20:22:03.568Z
Learning: In betaflight-configurator's msp.js, the bluetooth CRC bypass check at line 266 uses serial.protocol === "bluetooth". When protocol names are unified to "ble", this check must be updated to serial.protocol === "ble".
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: 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: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-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/port_handler.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/port_handler.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/port_handler.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/port_handler.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:
src/js/port_handler.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/port_handler.js
⏰ 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: Build / build
🔇 Additional comments (1)
src/js/port_handler.js (1)
176-193: LGTM! Improved permission flow with robust fallback handling.The changes properly handle all permission outcomes:
- If a specific port is granted, it's selected immediately (line 179 early return)
- If permission is granted but no specific port returned, the first available device is selected as fallback (lines 182-190)
- If no devices exist post-permission, it logs and resets to DEFAULT_PORT (lines 192-193)
This is a significant improvement over previous behavior and handles edge cases more gracefully.
|
|
🎉 Do you want to test this code? 🎉 |
haslinghuis
requested review from
McGiverGim,
VitroidFPV,
nerdCopter and
sugaarK
3 participants
Tested devices
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.