Add USB serial support for Android (Capacitor) #4698
Conversation
|
This comment was marked as outdated.
This comment was marked as outdated.
|
@coderabbitai - my bad left the debug for comparison - now removed. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1646 @haslinghuis, perfect! ✅ The debug console.log statements have been successfully removed from WebSerial.js. 🎉 Final Pre-Merge Assessment - ALL CLEAR✅ All Issues Resolved
📊 PR SummaryThis PR successfully adds Capacitor USB Serial support for Android:
🚀 Ready to MergeThe code is clean, well-structured, and follows the existing patterns in the codebase. Great work on the protocol abstraction and Android integration! 🎯 🧠 Learnings usedIf you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
![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 (3)
src/js/protocols/CapacitorSerial.js (2)
84-93: Consider adding hex string validation.If the plugin returns malformed data,
Number.parseIntwill returnNaNfor non-hexadecimal characters, producing an invalid byte array. While the plugin interface should guarantee valid hex strings, adding validation improves robustness.Apply this diff to add validation:
hexStringToUint8Array(hexString) { + if (!/^[0-9a-fA-F]*$/.test(hexString)) { + console.error(`${logHead} Invalid hex string:`, hexString); + return new Uint8Array(0); + } const length = hexString.length / 2; const uint8Array = new Uint8Array(length); for (let i = 0; i < length; i++) { uint8Array[i] = Number.parseInt(hexString.slice(i * 2, i * 2 + 2), 16); } return uint8Array; }
225-251: Add explicit null/undefined check for data parameter.While the try-catch will handle errors from
new Uint8Array(null), an explicit check provides clearer error messaging and prevents unnecessary plugin calls.Apply this diff:
async send(data, callback) { if (!this.isOpen) { console.error(`${logHead} Cannot send - not connected`); callback?.({ bytesSent: 0 }); return false; } + + if (data == null) { + console.error(`${logHead} Cannot send - data is null or undefined`); + callback?.({ bytesSent: 0 }); + return false; + } try { // Handle both Uint8Array and ArrayBuffer const uint8Array = data instanceof Uint8Array ? data : new Uint8Array(data);src/js/utils/checkCompatibility.js (1)
56-126: Solid refactoring of compatibility detection.The updated logic correctly handles both native Capacitor platforms and web browser contexts. The compatibility check properly short-circuits for native platforms and validates capability support for web browsers.
Optional nitpick: The error messages on lines 91, 95, and 99 are inconsistent—two mention "Web" (Web Bluetooth API, Web USB API) while line 95 says "Serial API" without the "Web" prefix. Consider standardizing to either include or exclude "Web" for all three for consistency.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/js/protocols/CapacitorSerial.js(1 hunks)src/js/utils/checkCompatibility.js(5 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4576
File: src/js/port_usage.js:17-23
Timestamp: 2025-08-22T16:43:20.901Z
Learning: In betaflight-configurator, the serial facade architecture requires accessing metrics like bitrate, bytesReceived, and bytesSent from serial._protocol rather than the top-level serial object. This change maintains compatibility with existing port utilization calculations that have been stable for over 11 years.
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4687
File: src/js/protocols/TauriSerial.js:299-310
Timestamp: 2025-11-06T17:26:50.201Z
Learning: In tauri-plugin-serialplugin Android development, the Android project must be initialized via `tauri android init` before the USB support patches (AndroidManifest.xml permissions, device_filter.xml, MainActivity.kt) can be applied. Without initialization, the plugin returns empty results from available_ports_direct() because the generated Android project structure and USB permissions don't exist yet.
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-10-25T21:16:32.474Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4379
File: src/js/protocols/TauriSerial.js:203-259
Timestamp: 2025-10-25T21:16:32.474Z
Learning: In TauriSerial (src/js/protocols/TauriSerial.js), the requestPermissionDevice() method is not needed and not invoked. Tauri automatically discovers serial devices through the constructor's loadDevices() and startDeviceMonitoring() calls, bypassing the browser permission model that WebSerial requires. Devices are auto-detected via a 1-second polling interval without user permission prompts.
Applied to files:
src/js/protocols/CapacitorSerial.jssrc/js/utils/checkCompatibility.js
📚 Learning: 2025-06-19T22:13:09.136Z
Learnt from: blckmn
Repo: betaflight/betaflight-configurator PR: 4521
File: src/js/protocols/WebSerial.js:148-151
Timestamp: 2025-06-19T22:13:09.136Z
Learning: In WebSerial.js, there's a timing issue where the cached `this.ports` array doesn't immediately reflect newly permitted devices after `requestPermissionDevice()` completes. The `getDevices()` method needs to refresh the device list from the browser API to return accurate data immediately following a permission request and user acceptance.
Applied to files:
src/js/protocols/CapacitorSerial.jssrc/js/utils/checkCompatibility.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/CapacitorSerial.jssrc/js/utils/checkCompatibility.js
📚 Learning: 2025-06-09T00:33:22.959Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-06-09T00:33:22.959Z
Learning: In the betaflight-configurator codebase, port paths use counter suffixes for serial and bluetooth ports (e.g., "serial1", "serial2", "bluetooth1", "bluetooth2") instead of using connectionId, which means simple string matching against protocol map keys won't work for protocol selection.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 Learning: 2025-09-19T20:42:20.332Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:42:20.332Z
Learning: Complex MSP duplicate handling fixes in Betaflight Configurator can cause infinite loading messages when changing tabs due to disruption of the callback resolution mechanism. Simple code-only duplicate detection (using this.callbacks.some((instance) => instance.code === code)) is the safer approach that preserves tab switching functionality.
Applied to files:
src/js/protocols/CapacitorSerial.jssrc/js/utils/checkCompatibility.js
📚 Learning: 2025-09-19T20:41:44.286Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4510
File: src/js/msp.js:384-391
Timestamp: 2025-09-19T20:41:44.286Z
Learning: When fixing MSP duplicate handling in Betaflight Configurator, avoid complex changes to callback resolution mechanisms as they can break tab switching functionality. Simple duplicate detection based on code and payload size is safer than complex requestKey-based approaches.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 Learning: 2025-11-06T17:26:50.201Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4687
File: src/js/protocols/TauriSerial.js:299-310
Timestamp: 2025-11-06T17:26:50.201Z
Learning: In tauri-plugin-serialplugin Android development, the Android project must be initialized via `tauri android init` before the USB support patches (AndroidManifest.xml permissions, device_filter.xml, MainActivity.kt) can be applied. Without initialization, the plugin returns empty results from available_ports_direct() because the generated Android project structure and USB permissions don't exist yet.
Applied to files:
src/js/protocols/CapacitorSerial.js
📚 Learning: 2025-05-14T21:39:16.388Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 0
File: :0-0
Timestamp: 2025-05-14T21:39:16.388Z
Learning: On Android Chrome browsers, Web Bluetooth API is supported from version 133+ (requires Android 6.0+) and Web USB API is supported from version 136+, but Web Serial API is not supported. The Betaflight Configurator should detect and use available APIs on Android rather than requiring all three.
Applied to files:
src/js/utils/checkCompatibility.js
📚 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/utils/checkCompatibility.js
📚 Learning: 2025-08-27T22:07:26.353Z
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4579
File: src/js/data_storage.js:9-9
Timestamp: 2025-08-27T22:07:26.353Z
Learning: In the betaflight-configurator CalVer refactoring, the generateVirtualApiVersions function in src/js/utils/common.js works correctly despite the apparent mathematical issue with bounds logic where compareVersions.minor() returns different ranges for API_VERSION_MAX_SUPPORTED (25.12.0) vs API_VERSION_ACCEPTED (1.44.0).
Applied to files:
src/js/utils/checkCompatibility.js
🔇 Additional comments (3)
src/js/protocols/CapacitorSerial.js (1)
1-284: Overall implementation looks solid.The CapacitorSerialProtocol implementation follows the established patterns from WebSerial and TauriSerial. Event listener management, error handling, and state management are properly implemented. Past review concerns have been addressed with the addition of the cleanup() method and removal of duplicate event dispatches.
src/js/utils/checkCompatibility.js (2)
7-8: Good modernization of navigator access.Using
globalThis.navigatorinstead ofwindow.navigatorimproves compatibility across different JavaScript environments (browsers, workers, etc.).
128-139: Correct platform-specific serial support detection.The logic properly distinguishes between Android native (Capacitor plugin) and web browser contexts. The
isAndroid()check only returns true for native Capacitor platforms, not Android Chrome web browsers, so the fallback tonavigator.serialworks correctly for web contexts.
haslinghuis
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
patches/capacitor-plugin-usb-serial+0.0.6.patch (1)
304-314: Hex-based write implementation looks good.The conversion from text-based transmission (with "\r\n" suffix) to hex-encoded binary transmission correctly supports binary protocols like MSP. The comment clearly explains the rationale.
Consider adding a null check for defensive programming:
if (str.isEmpty()) { throw new Error("can't send empty string to device", new Throwable("EMPTY_STRING")); } + if (str == null) { + throw new Error("can't send null to device", new Throwable("NULL_INPUT")); + }Though the Capacitor bridge likely validates input, explicit null handling improves robustness.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
patches/capacitor-plugin-usb-serial+0.0.6.patch(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: haslinghuis
Repo: betaflight/betaflight-configurator PR: 4687
File: src/js/protocols/TauriSerial.js:299-310
Timestamp: 2025-11-06T17:26:50.201Z
Learning: In tauri-plugin-serialplugin Android development, the Android project must be initialized via `tauri android init` before the USB support patches (AndroidManifest.xml permissions, device_filter.xml, MainActivity.kt) can be applied. Without initialization, the plugin returns empty results from available_ports_direct() because the generated Android project structure and USB permissions don't exist yet.
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: 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.
🔇 Additional comments (1)
patches/capacitor-plugin-usb-serial+0.0.6.patch (1)
316-343: Excellent fix for binary protocol support.The replacement of NMEA sentence assembly with immediate hex-string forwarding correctly addresses the core issue: binary protocols like MSP don't use newline delimiters, so the original code would never trigger the callback. The "FIXED" comment clearly explains the problem and solution.
The implementation:
- Immediately forwards all received data as hex strings
- Removes buffering logic that was appropriate only for text-based NMEA protocol
- Preserves error handling via
updateReadDataError- Aligns with the hex-based
write()method for consistent binary data flow
VitroidFPV
left a comment
VitroidFPV
left a comment
There was a problem hiding this comment.
Tested with SEQUREH743V2 and IFLIGHT_BLITZ_F435. Once installed via adb I can connect an FC via USB C-C cable and have it detected. Connects fine and I can see the 3d model react to real movements. Approving to get this finished, further issues can be adressed in future PRs
|
|
Preview URL: https://pr4698.betaflight-app-preview.pages.dev |
blckmn
left a comment
blckmn
left a comment
There was a problem hiding this comment.
This will need to wait until we release.
3 participants
Summary by CodeRabbit
New Features
Improvements
Chores