Android serial: full overhaul (lifecycle FSM, async write pump, strategy backend)#14358
Draft
HTRamsey wants to merge 1 commit into
Draft
Android serial: full overhaul (lifecycle FSM, async write pump, strategy backend)#14358HTRamsey wants to merge 1 commit into
HTRamsey wants to merge 1 commit into
Conversation
github-actions
Bot
added
Platform: Android
github_actions
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors QGroundControl’s Android USB serial integration by splitting JNI/USB lifecycle responsibilities into dedicated Java/C++ components, adding a typed exception channel for consistent teardown/error mapping, and improving logging/diagnostics to support reliable hot-unplug/reconnect behavior.
Changes:
- Replaces the Android USB serial Java stack with a new manager/enumerator/registry/lifecycle design plus a C++
QAndroidSerialEngineJNI bridge and receiver interface. - Adds JNI log forwarding from Java into Qt logging and expands firmware-upgrade UX/diagnostics (Android guidance + bootloader verbose hex/timeout logging).
- Updates Android USB device filtering, Proguard keep rules, and Java unit tests to match the new serial package layout.
Reviewed changes
Copilot reviewed 57 out of 57 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Vehicle/VehicleSetup/VehicleConfigView.qml | Shows Firmware setup option on non-iOS platforms. |
| src/Vehicle/VehicleSetup/FirmwareUpgrade.qml | Adds Android-specific bootloader permission timing note to status log. |
| src/Vehicle/VehicleSetup/Bootloader.cc | Adds verbose TX/RX hex preview + richer timeout diagnostics and sync attempt logging. |
| src/Vehicle/Vehicle.h | Adds (debug) benchmark slot declaration. |
| src/Vehicle/Vehicle.cc | Adjusts param lookup logic and adds (debug) write-burst benchmark implementation. |
| src/Comms/SerialLink.h | Adds gated error emission helper declaration to reduce UI error floods. |
| src/Comms/SerialLink.cc | Adds write-buffer soft cap and routes repeated write errors through a once-per-cycle gate. |
| src/Android/qtandroidserialport/qtserialportversion.h | Removes generated QtSerialPort version header from vendored Android port. |
| src/Android/qtandroidserialport/qtserialportexports.h | Removes generated export-macro header from vendored Android port. |
| src/Android/qtandroidserialport/qserialportinfo.h | Drops export macro dependency for Android-local QSerialPortInfo. |
| src/Android/qtandroidserialport/qserialportinfo.cpp | Removes large upstream doc blocks (keeps implementation). |
| src/Android/qtandroidserialport/qserialportinfo_android.cpp | Updates Android serial-port-info logging category name. |
| src/Android/qtandroidserialport/qserialportglobal.h | Removes unused global header previously providing export macros. |
| src/Android/qtandroidserialport/qserialport.h | Adds Android-local API parity fields/methods (write buffer size, settings restore, sendBreak). |
| src/Android/qtandroidserialport/qserialport.cpp | Implements added API surface and adjusts clear/read paths for new engine staging. |
| src/Android/qtandroidserialport/qserialport_p.h | Reworks private implementation around QAndroidSerialEngine + receiver callbacks and write draining. |
| src/Android/qtandroidserialport/qandroidserialenginereceiver_p.h | Introduces receiver interface decoupling JNI engine from QSerialPortPrivate. |
| src/Android/qtandroidserialport/qandroidserialengine.cpp | Adds new JNI engine: token registry, callbacks, marshaling, direct-buffer write path. |
| src/Android/qtandroidserialport/qandroidserialengine_p.h | Declares engine API + read staging/waiting mechanics and JNI callback hooks. |
| src/Android/qtandroidserialport/CMakeLists.txt | Adds new engine sources/headers; removes unused generated headers from build. |
| src/Android/CMakeLists.txt | Adds AndroidLogSink sources to the Android target. |
| src/Android/AndroidSerial.h | Simplifies serial constants/types and switches to initialize() entry point. |
| src/Android/AndroidLogSink.h | Declares native Java-log-to-Qt bridge registration and log category. |
| src/Android/AndroidLogSink.cc | Implements JNI native log sink mapping Java levels into Qt categories. |
| src/Android/AndroidInterface.cc | Removes legacy JNI debug/warn log forwarders from AndroidInterface. |
| src/Android/AndroidInit.cc | Registers AndroidLogSink + switches serial init to AndroidSerial::initialize(); removes JNI unload cleanup. |
| android/src/test/java/org/mavlink/qgroundcontrol/serial/SerialUnitTest.java | Adds unit tests for device-port parsing and handle mapping in new serial package. |
| android/src/test/java/org/mavlink/qgroundcontrol/QGCUsbSerialManagerTest.java | Removes old unit tests tied to the pre-refactor manager API/package. |
| android/src/org/mavlink/qgroundcontrol/serial/UsbSerialLifecycle.java | Implements per-port open/close + IO manager lifecycle and pump setup/teardown. |
| android/src/org/mavlink/qgroundcontrol/serial/UsbSerialIoBridge.java | Provides stateless JNI-exposed per-port operations (write, params, lines, flow control, purge, break). |
| android/src/org/mavlink/qgroundcontrol/serial/UsbSerialEnumerator.java | Owns driver tracking/probing and deviceName/#pN parsing/building. |
| android/src/org/mavlink/qgroundcontrol/serial/UsbPortInfo.java | Adds record type for structured port info returned to native via JNI. |
| android/src/org/mavlink/qgroundcontrol/serial/UsbDeviceRegistry.java | Adds handle→resources registry with atomic snapshot updates and safe extraction for cleanup. |
| android/src/org/mavlink/qgroundcontrol/serial/SerialConstants.java | Centralizes shared constants (chunk sizes, exception kinds, FTDI control constants). |
| android/src/org/mavlink/qgroundcontrol/serial/RateMonitor.java | Adds lightweight rolling-window rate logging for benchmarking read/write paths. |
| android/src/org/mavlink/qgroundcontrol/serial/QGCUsbSerialProber.java | Implements prober that prefers D2XX-backed FTDI when available and logs probe decisions. |
| android/src/org/mavlink/qgroundcontrol/serial/QGCUsbSerialManager.java | Introduces new singleton manager coordinating permissions, enumeration, lifecycle, and JNI dispatch. |
| android/src/org/mavlink/qgroundcontrol/serial/QGCUsbPermissionHandler.java | Adds robust receiver-based USB permission and attach/detach handling (incl. OEM quirks). |
| android/src/org/mavlink/qgroundcontrol/serial/QGCSerialListener.java | Implements IO-thread listener emitting chunked data + typed exceptions to native. |
| android/src/org/mavlink/qgroundcontrol/serial/QGCFtdiSerialDriver.java | Adds D2XX-backed FTDI driver/port implementation and wiring for baud-aware throttling. |
| android/src/org/mavlink/qgroundcontrol/serial/NativeDataEmitter.java | Adds pooled direct-ByteBuffer emitter for JNI read callbacks. |
| android/src/org/mavlink/qgroundcontrol/serial/BoundedPool.java | Adds bounded, size-aware pooling utility used by emitters and write scratch buffers. |
| android/src/org/mavlink/qgroundcontrol/serial/AsyncUsbWritePump.java | Adds async USB write pump supporting bulkTransfer and D2XX modes with backpressure/throttling. |
| android/src/org/mavlink/qgroundcontrol/QGCUsbSerialProber.java | Removes obsolete prober (old package/path) replaced by new serial package implementation. |
| android/src/org/mavlink/qgroundcontrol/QGCUsbId.java | Removes obsolete USB VID/PID constants class (old package/path). |
| android/src/org/mavlink/qgroundcontrol/QGCNativeLogSink.java | Adds Java→native log bridge with lazy JNI availability check. |
| android/src/org/mavlink/qgroundcontrol/QGCLogger.java | Mirrors Java logs into native Qt logging and adds Supplier-based lazy message overloads. |
| android/src/org/mavlink/qgroundcontrol/QGCFtdiSerialDriver.java | Removes old FTDI driver implementation replaced by new D2XX-focused driver in serial package. |
| android/src/org/mavlink/qgroundcontrol/QGCFtdiDriver.java | Removes old D2XX wrapper replaced by new consolidated FTDI driver/port implementation. |
| android/src/org/mavlink/qgroundcontrol/QGCActivity.java | Switches USB serial manager lifecycle to create/destroy singleton; removes legacy native log JNI stubs. |
| android/res/xml/device_filter.xml | Replaces catch-all USB filter with explicit VID/PID list + expanded documentation. |
| android/proguard-rules.pro | Updates keep rules for new serial package and native log sink. |
| AGENTS.md | Adds Android install/logcat workflow instructions for developers/agents. |
| .github/mcp-config.json | Adds MCP server configuration for Qt docs / Qt Creator integrations. |
Comment on lines
+1066
to
+1068
| QString armingRequireParam("ARMING_REQUIRE"); | ||
| return _parameterManager->parameterExists(ParameterManager::defaultComponentId, armingRequireParam) && | ||
| _parameterManager->getParameter(ParameterManager::defaultComponentId, armingRequireParam)->rawValue().toInt() == 0; | ||
| Fact *param = _parameterManager->getParameter(ParameterManager::defaultComponentId, armingRequireParam); | ||
| return param && param->rawValue().toInt() == 0; |
Comment on lines
+1316
to
+1320
| // ============================================================================ | ||
| // BENCHMARK INSTRUMENTATION (debug-only). Fires a 10 Hz × 850-msg heartbeat | ||
| // burst on the primary link to saturate write-path drain. Started after the | ||
| // 5th heartbeat; runs until Vehicle is destroyed (timer parented to `this`). | ||
| // To strip: remove this method, its declaration in Vehicle.h, and the |
| void _handlePing (LinkInterface* link, mavlink_message_t& message); | ||
| void _handleHomePosition (mavlink_message_t& message); | ||
| void _handleHeartbeat (mavlink_message_t& message); | ||
| void _benchTriggerWriteBurstStress (); // DEBUG: write-path stress benchmark; remove with body in Vehicle.cc. |
Comment on lines
+69
to
+71
| static QReadWriteLock s_ptrLock; | ||
| static QHash<jlong, QAndroidSerialEngine*> s_tokenToEngine; | ||
| static QHash<QAndroidSerialEngine*, jlong> s_engineToToken; |
Comment on lines
+124
to
+130
| final byte[] buf; | ||
| if (offset == 0 && data.length == length) { | ||
| buf = data; | ||
| } else { | ||
| buf = new byte[length]; | ||
| System.arraycopy(data, offset, buf, 0, length); | ||
| } |
Comment on lines
+159
to
+162
| bool _portOp(Op&& op, QSerialPort::SerialPortError errorCode, const QString& failMsg) | ||
| { | ||
| if (!_engine) return false; | ||
| const bool result = op(_engine.get()); |
Comment on lines
+76
to
+77
| QGCSerialDriver:V QGCFtdiSerialDriver:V QGCSerialListener:V UsbSerialIoBridge:V \ | ||
| UsbDeviceRegistry:V QGCUsbId:V QGCUsbSerialProber:V '*:S' |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (30.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #14358 +/- ##
=========================================
Coverage ? 25.36%
=========================================
Files ? 766
Lines ? 65617
Branches ? 30345
=========================================
Hits ? 16647
Misses ? 37246
Partials ? 11724
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Contributor
Build ResultsPlatform Status
Some builds failed. Pre-commit
Pre-commit hooks: 4 passed, 48 failed, 7 skipped. Updated: 2026-05-18 16:09:02 UTC • Triggered by: Android |
HTRamsey
force-pushed
the
fix/android-serial-overhaul
branch
from
87f74f1 to
0ea1e64
Compare
HTRamsey
changed the title
…egy backend) Replaces the vendored QSerialPort backport with a standalone QGCAndroidSerialPort that wraps a strategy engine (IQSerialPortEngine). Production uses QAndroidSerialEngine; host tests use MockQSerialPortEngine — same contract, no JNI dependency. Java side: - Explicit port lifecycle FSM (UsbSerialLifecycle) replacing scattered state. - Consolidated USB permission handling (QGCUsbPermissionHandler). - AsyncUsbWritePump: bounded queue + N workers per port + per-baud token-bucket throttle. forBulkTransfer (N=4) for CDC-ACM/CP210x/CH34x/mik3y FTDI; forD2xx (N=1) for D2XX-backed FTDI. Ownership-transfer submit() — pump owns the buffer until onComplete fires, eliminating a per-write heap copy. - BoundedPool for write-scratch byte[] reuse. - READ_QUEUE_DEPTH=3 (was 2) to ride out GC pauses at high baud. - Typed Java exception channel surfaces as QGCSerialPortError on the C++ side instead of opaque IOException strings. - D2XX gated off by default (ENABLE_D2XX=false). Tracks mavlink#14146: canOpenViaD2XX() returns true on Android 14 / OneUI 6 then the actual open fails. VCP-mode FtdiSerialDriver covers the same chips. C++ side: - JNIEnv caching on the hot write path (static getJniEnv + checkAndClearExceptions) — drops a QJniEnvironment construction per call. - Read offset sentinel in QGCAndroidSerialPort: readData advances an offset and compacts only when half-consumed, instead of erase-from-front on every drain. - QGCSerialPortAdapter: strategy-pattern backend (AndroidSerialBackend always, HostSerialBackend on !Q_OS_ANDROID). Modeled on Qt's QNetworkAccessBackend. Collapses ~32 Q_OS_ANDROID conditionals down to 4 (include guard, host error mapper, host backend class, factory dispatch). - Batched setSerialParameters(baud, dataBits, stopBits, parity) — collapses what was 4 sequential single-param JNI hops into 1. Wired through Bootloader, SerialLink, GPSProvider. Tests: - JVM unit tests: AsyncUsbWritePumpTest (7), BoundedPoolTest (4). - C++ unit tests via MockQSerialPortEngine: QGCAndroidSerialPortTest covers lifecycle, setters (cached-while-closed + applied-while-open), batched parameter setter, exception → error mapping, expectClosure suppression. - QGCSerialPortAdapterTest exercises the strategy dispatch.
HTRamsey
force-pushed
the
fix/android-serial-overhaul
branch
from
0ea1e64 to
f119f14
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Refactors the Android USB serial stack — typed exception channel, FTDI/D2XX cleanup, and removal of the dead
USBBoardInfo.jsonasset path. Full commit message captures the per-area details.Validated end-to-end on a Samsung Tab S10 (Android 15, SDK 35) over a long testing session covering:
kind=1(Resource) exception → autoconnect re-attach with fresh deviceIdkind=1exception + GPSManager re-configPump-level errors observed across all tests: 0. Native heap stable post-warmup on every soak.
Test plan