fix(Android): USB-host serial stack overhaul (lifecycle FSM, async write pump, hot-plug)#14536
Open
HTRamsey wants to merge 1 commit into
Open
fix(Android): USB-host serial stack overhaul (lifecycle FSM, async write pump, hot-plug)#14536HTRamsey wants to merge 1 commit into
HTRamsey wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR overhauls QGroundControl’s serial stack with a platform-neutral QGCSerialPort abstraction, a new Comms/Serial module layout, and a significantly reworked Android USB-host backend to improve lifecycle correctness, async I/O, and hot-plug robustness. It also updates autoconnect behavior and adds host (CTest) + Android (JUnit) test coverage for key parsing/lifecycle pieces.
Changes:
- Introduces a unified
QGCSerialPort+SerialPlatformseam and refactors serial link code to use it across host and Android. - Reworks Android USB serial management (JNI bridge, permission/attach receivers, lifecycle handling) and adds Java-side log bridging into Qt categories.
- Updates UI/settings/autoconnect behavior for hot-plug + per-port baud lists; adds new unit tests.
Reviewed changes
Copilot reviewed 99 out of 100 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| test/Comms/QGCSerialPortInfoTest.h | Adds new unit-test slots |
| test/Comms/QGCSerialPortInfoTest.cc | Adds JSON schema + Linux filtering tests |
| test/Comms/NmeaSerialDeviceTest.h | New NMEA device unit test header |
| test/Comms/NmeaSerialDeviceTest.cc | New NMEA device unit test implementation |
| test/Comms/HostSerialPortTest.h | New HostSerialPort unit test header |
| test/Comms/HostSerialPortTest.cc | New HostSerialPort unit test implementation |
| test/Comms/CMakeLists.txt | Registers new Comms unit tests |
| src/Vehicle/VehicleSetup/VehicleConfigView.qml | Shows Firmware button on Android (not iOS) |
| src/Vehicle/VehicleSetup/FirmwareUpgradeController.h | Switches to QGCSerialPortInfo |
| src/Vehicle/VehicleSetup/FirmwareUpgradeController.cc | Updates signature to QGCSerialPortInfo |
| src/Vehicle/VehicleSetup/FirmwareUpgrade.qml | Adds Android flashing note text |
| src/Vehicle/VehicleSetup/Bootloader.h | Switches bootloader to QGCSerialPort* |
| src/Vehicle/VehicleSetup/Bootloader.cc | Uses SerialPlatform + adds verbose TX/RX logging |
| src/Vehicle/Vehicle.h | Adds benchmark hook declaration |
| src/Vehicle/Vehicle.cc | Adds benchmark method + parameter gating tweaks |
| src/Utilities/SDL/CMakeLists.txt | Android linker page-size option |
| src/Utilities/Logging/QGCLoggingCategoryManager.cc | Uses Qt default category name dynamically |
| src/GPS/SerialGPSTransport.h | Switches to QGCSerialPort + config bundle |
| src/GPS/SerialGPSTransport.cc | Uses SerialPlatform + QGCSerialPortError mapping |
| src/Comms/Serial/USBBoardInfo.json | Removes Android-only broad fallback |
| src/Comms/Serial/SerialPlatform.h | New platform hook API for serial |
| src/Comms/Serial/SerialPlatform_host.cc | Host implementations of platform hooks |
| src/Comms/Serial/SerialPlatform_android.cc | Android implementations of platform hooks |
| src/Comms/Serial/SerialLink.h | Refactors config + worker to QGCSerialPort |
| src/Comms/Serial/SerialLink.cc | Worker-thread model + unified error handling |
| src/Comms/Serial/QGCSerialPortTypes.h | New shared enums + SerialPortConfig |
| src/Comms/Serial/QGCSerialPortInfo.h | New value-type port descriptor |
| src/Comms/Serial/QGCSerialPortInfo.cc | New enumerator + JSON parsing logic |
| src/Comms/Serial/QGCSerialPort.h | New unified serial QIODevice interface |
| src/Comms/Serial/NmeaSerialDevice.h | New worker-threaded NMEA source device |
| src/Comms/Serial/NmeaSerialDevice.cc | Implements sequential read-only adapter |
| src/Comms/Serial/HostSerialPort.h | New QSerialPort-backed QGCSerialPort impl |
| src/Comms/Serial/HostSerialPort.cc | Implements host serial errors/presence checks |
| src/Comms/Serial/CMakeLists.txt | New Comms/Serial submodule build rules |
| src/Comms/QGCSerialPortInfo.h | Removed (moved to Comms/Serial) |
| src/Comms/QGCSerialPortInfo.cc | Removed (moved to Comms/Serial) |
| src/Comms/LinkManager.h | Android autoconnect timing + new API |
| src/Comms/LinkManager.cc | Hot-plug callback + NMEA source via SerialPlatform |
| src/Comms/CMakeLists.txt | Adds Serial subdirectory |
| src/AutoPilotPlugins/APM/APMAutoPilotPlugin.cc | BlackCube check via SerialConfiguration |
| src/AppSettings/SerialSettings.qml | Baud model refresh per selected port |
| src/AppSettings/NmeaGpsSettings.qml | Rebuild port list on hot-plug |
| src/Android/qtandroidserialport/qtserialportversion.h | Removed vendored QtSerialPort header |
| src/Android/qtandroidserialport/qtserialportexports.h | Removed vendored QtSerialPort header |
| src/Android/qtandroidserialport/qserialportinfo.h | Removed vendored QtSerialPort header |
| src/Android/qtandroidserialport/qserialportinfo.cpp | Removed vendored QtSerialPort source |
| src/Android/qtandroidserialport/qserialportinfo_p.h | Removed vendored QtSerialPort header |
| src/Android/qtandroidserialport/qserialportinfo_android.cpp | Removed vendored QtSerialPort source |
| src/Android/qtandroidserialport/qserialportglobal.h | Removed vendored QtSerialPort header |
| src/Android/qtandroidserialport/qserialport.h | Removed vendored QtSerialPort header |
| src/Android/qtandroidserialport/qserialport_p.h | Removed vendored QtSerialPort header |
| src/Android/qtandroidserialport/CMakeLists.txt | Stops building vendored QtSerialPort |
| src/Android/CMakeLists.txt | Switches to new Android serial backend files |
| src/Android/AndroidSerialPortRegistry.h | New JNI token→port registry |
| src/Android/AndroidSerialPortRegistry.cc | Implements registry with RW lock |
| src/Android/AndroidSerialPortPrivate.h | New AndroidSerialPort d-pointer API |
| src/Android/AndroidSerialPort.h | New AndroidSerialPort public API |
| src/Android/AndroidSerialPort.cc | New Android serial impl (async writer, lifecycle) |
| src/Android/AndroidSerial.h | Removed old Android serial API |
| src/Android/AndroidLogSink.h | New Java→Qt logging bridge API |
| src/Android/AndroidLogSink.cc | Registers native log sink + category mapping |
| src/Android/AndroidInterface.h | Simplifies JNI types + declarations |
| src/Android/AndroidInterface.cc | Updates native method registration + safety |
| src/Android/AndroidInit.cc | Registers new JNI hooks (serial + log sink) |
| android/src/test/java/org/mavlink/qgroundcontrol/serial/UsbSerialEnumeratorParsingTest.java | New parsing unit tests |
| android/src/test/java/org/mavlink/qgroundcontrol/serial/SerialConstantsTest.java | New constants/unit tests |
| android/src/test/java/org/mavlink/qgroundcontrol/serial/QGCSerialPortLifecycleTest.java | New lifecycle FSM tests |
| android/src/test/java/org/mavlink/qgroundcontrol/serial/QGCSerialPortBridgeTest.java | New NativeBridge routing tests |
| android/src/test/java/org/mavlink/qgroundcontrol/serial/D2xxLibraryTest.java | New D2XX helper tests |
| android/src/test/java/org/mavlink/qgroundcontrol/QGCUsbSerialManagerTest.java | Removed old manager tests |
| android/src/org/mavlink/qgroundcontrol/serial/UsbSerialEnumerator.java | New driver tracking + parsing |
| android/src/org/mavlink/qgroundcontrol/serial/UsbPortInfo.java | New JNI-friendly device info record |
| android/src/org/mavlink/qgroundcontrol/serial/UsbPermissionManager.java | New permission handling component |
| android/src/org/mavlink/qgroundcontrol/serial/UsbAttachDetachReceiver.java | New attach/detach receiver |
| android/src/org/mavlink/qgroundcontrol/serial/SerialConstants.java | New shared serial constants |
| android/src/org/mavlink/qgroundcontrol/serial/QGCUsbSerialProber.java | New prober with D2XX preference |
| android/src/org/mavlink/qgroundcontrol/serial/QGCFtdiSerialDriver.java | New D2XX-backed driver wrapper |
| android/src/org/mavlink/qgroundcontrol/serial/DriverCapabilities.java | New driver capability bundle |
| android/src/org/mavlink/qgroundcontrol/serial/D2xxLibrary.java | New D2XX lifecycle/helpers |
| android/src/org/mavlink/qgroundcontrol/QGCUsbSerialProber.java | Removed old prober |
| android/src/org/mavlink/qgroundcontrol/QGCUsbId.java | Removed old USB ID list |
| android/src/org/mavlink/qgroundcontrol/QGCNativeLogSink.java | New Java→native log bridge |
| android/src/org/mavlink/qgroundcontrol/QGCLogger.java | Mirrors Java logs into native + suppliers |
| android/src/org/mavlink/qgroundcontrol/QGCFtdiDriver.java | Removed old D2XX driver impl |
| android/src/org/mavlink/qgroundcontrol/QGCForegroundService.java | New connected-device FGS helper |
| android/src/org/mavlink/qgroundcontrol/QGCActivity.java | Manager lifecycle gating + notification permission |
| android/proguard-rules.pro | Updates keep rules for new packages |
| android/build.gradle | JVM unit test defaults enabled |
| android/AndroidManifest.xml | Adds FGS + notifications permissions + service |
| AGENTS.md | Adds Android install/logcat/test workflow |
Comment on lines
+87
to
+103
| bool HostSerialPort::open(QIODevice::OpenMode mode) | ||
| { | ||
| if (!_port.open(mode)) { | ||
| // QSerialPort emits errorOccurred (→ _setError) synchronously on failure; only synthesize one if it didn't. | ||
| if (_port.error() == QSerialPort::NoError) { | ||
| _setError(QGCSerialPortError::OpenFailed, tr("Failed to open serial port")); | ||
| } | ||
| return false; | ||
| } | ||
| _port.setReadBufferSize(kSerialRxBufferCapBytes); | ||
| QIODevice::open(mode); | ||
| #ifndef Q_OS_ANDROID | ||
| _presenceMissCount = 0; | ||
| _presenceTimer->start(); | ||
| #endif | ||
| return true; | ||
| } |
Comment on lines
253
to
270
| @@ -232,6 +264,8 @@ bool Bootloader::_write(const uint8_t* data, qint64 maxSize) | |||
| return false; | |||
| } | |||
|
|
|||
| qCDebug(FirmwareUpgradeVerboseLog).noquote() | |||
| << QStringLiteral("TX n=%1 hex=%2").arg(maxSize).arg(_hexPreview(data, maxSize)); | |||
| return true; | |||
| } | |||
Comment on lines
+42
to
+50
| _config.baud = 9600; | ||
| _config.dataBits = QGCDataBits::Data8; | ||
| _config.parity = QGCParity::None; | ||
| _config.stopBits = QGCStopBits::One; | ||
| _config.flowControl = QGCFlowControl::None; | ||
| if (!_serial->reconfigure(_config)) { | ||
| qCWarning(SerialGPSTransportLog) << "GPS: Failed to configure Serial Device" << _device << _serial->errorString(); | ||
| return false; | ||
| } |
Comment on lines
+73
to
+87
| bool NmeaSerialDevice::open(OpenMode mode) | ||
| { | ||
| Q_UNUSED(mode); | ||
| Q_D(NmeaSerialDevice); | ||
| if (isOpen()) { | ||
| return false; | ||
| } | ||
|
|
||
| d->_reader = new NmeaSerialReader(d->_portName, d->_baud, nullptr); | ||
| // Cross-thread (worker emits, this lives on the GUI thread) -> auto-queued. | ||
| (void) connect(d->_reader, &NmeaSerialReader::dataReceived, this, &NmeaSerialDevice::_appendFromReader); | ||
| d->_reader->start(); | ||
|
|
||
| return QIODevice::open(QIODeviceBase::ReadOnly); | ||
| } |
Comment on lines
853
to
855
| void _handleHeartbeat (mavlink_message_t& message); | ||
| void _benchTriggerWriteBurstStress (); // DEBUG: write-path stress benchmark; remove with body in Vehicle.cc. | ||
| void _handleCurrentMode (mavlink_message_t& message); |
| } | ||
| }); | ||
| s_burstTimer->start(); | ||
| } |
HTRamsey
force-pushed
the
fix/android-serial-overhaul
branch
2 times, most recently
from
0cc5d57 to
b4c2cb8
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #14536 +/- ##
==========================================
+ Coverage 25.47% 30.55% +5.08%
==========================================
Files 769 787 +18
Lines 65912 67541 +1629
Branches 30495 31291 +796
==========================================
+ Hits 16788 20635 +3847
+ Misses 37285 32927 -4358
- Partials 11839 13979 +2140
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 375 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Contributor
Build ResultsPlatform Status
All builds passed. Pre-commit
Pre-commit hooks: 2 passed, 76 failed, 7 skipped. Test Resultslinux-coverage: 102 passed, 0 skipped Code CoverageCoverage: 65.0% No baseline available for comparison Artifact Sizes
Updated: 2026-06-16 00:08:57 UTC • Commit: b4c2cb8 • Triggered by: Linux |
HTRamsey
force-pushed
the
fix/android-serial-overhaul
branch
from
b4c2cb8 to
fd94aab
Compare
github-actions
Bot
added
github_actions
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| - name: Cache Gradle (Android tests) | ||
| if: ${{ matrix.host == 'linux' || matrix.emulator }} | ||
| uses: gradle/actions/setup-gradle@v6 |
|
|
||
| - name: Publish Unit Test Report | ||
| if: ${{ always() && matrix.host == 'linux' }} | ||
| uses: mikepenz/action-junit-report@v5 |
HTRamsey
force-pushed
the
fix/android-serial-overhaul
branch
14 times, most recently
from
19f195d to
c0839ad
Compare
HTRamsey
force-pushed
the
fix/android-serial-overhaul
branch
4 times, most recently
from
9b5efd4 to
1d64c3e
Compare
Rework the Android USB-serial backend for correct lifecycle, async I/O, and robust hot-plug behavior. Device-verified on an SM-T630 with a Cube Orange+ (connect, parameter download, repeated unplug/replug including write-in-flight detach, and clean app close). Highlights: - Lifecycle FSM (REGISTERED -> CONFIGURED -> CLOSING -> CLOSED) with all transitions centralized and guarded; single-shot close + unregister. - Async write pump: a dedicated writer thread drains a queue and acks bytes to C++ per sub-write, keeping outstanding-byte accounting in sync on partial writes. - NativeBridge JNI seam isolating native callbacks for testability; JUnit suite covering the FSM table and bridge routing. - Comms/Serial layout reorganized; QGCSerialPort/SerialLink/SerialWorker split with a worker-on-QThread model. Lifecycle/hot-plug fixes: - Manager no longer torn down during startup: drop the Qt qAddPostRoutine that destroyed the Activity-owned singleton when the startup-phase QGCApplication is destroyed; gate QGCActivity.onDestroy on m_instance==this && isFinishing(). - No crash on unplug with a write in flight: run the blocking USB write outside lifecycleLock so a wedged write can't starve teardown or make cancelPendingWrites unreachable; ~SerialLink waits for a clean worker exit instead of terminate()-ing a JNI-parked thread (which destroyed a running QThread -> qFatal). - No reconnect strand: the same lock fix prevents a port stuck in CLOSING blocking AutoConnect reopen; openJavaPort's null path now setError(OpenFailed) instead of leaving "No error". - Composite-port filter now works on Android: availableDevices() sets hasVendor/ProductIdentifier so LinkManager::_filterCompositePorts can dedup a board's secondary CDC port (e.g. the Cube's second ACM). Without it both ports auto-connected and the unserviced one churned open->write-fail->teardown every ~21s. - Bounded writer-join (WRITER_JOIN_TIMEOUT_MS) so close() can't outlast the C++ disconnect timeout; a parked write is unblocked by the following closePortLocked() rather than waited out, keeping teardown and app exit fast. - Stop forwarding self-issued permission results to Qt (no spurious "no valid pending permission request" warning). Autoconnect latency (Android): - Faster initial connect: USB attach/permission-granted now kicks an immediate autoconnect pass (Java -> JNI native -> LinkManager) instead of waiting up to a full poll interval; SerialPlatform gains a setDevicesChangedCallback seam (host no-op). - Tuned the per-port settle for Android: poll 1000->500ms and connect on the 3rd pass (~1s) rather than the desktop ~2s. The OS fully enumerates and permission-grants the device before the poll sees it, so the only wait needed covers the board's own enumerate->firstMAVLink window (~440ms measured on a Cube), opening past the boot tail. Warm reconnect after the device stabilizes drops from ~2-3s to ~210ms.
HTRamsey
force-pushed
the
fix/android-serial-overhaul
branch
from
1d64c3e to
949efdb
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
Reworks the Android USB-serial backend for correct lifecycle, async I/O, and robust hot-plug behavior, and reorganizes the
Comms/Seriallayout. Device-verified on an SM-T630 with a Cube Orange+ (connect, parameter download, repeated unplug/replug including write-in-flight detach, and clean app close).Highlights
REGISTERED → CONFIGURED → CLOSING → CLOSED) with all transitions centralized and guarded; single-shot close + unregister.QGCSerialPort/SerialLink/SerialWorkersplit with a worker-on-QThread model; host port backed byQSerialPort(HostSerialPort).Lifecycle / hot-plug fixes
qAddPostRoutinethat destroyed the Activity-owned singleton; gatedQGCActivity.onDestroyonm_instance==this && isFinishing()).lifecycleLock;~SerialLinkwaits for a clean worker exit instead ofterminate()-ing a JNI-parked thread.CLOSINGblocking AutoConnect reopen;openJavaPort's null path nowsetError(OpenFailed)instead of leaving "No error".availableDevices()setshasVendor/ProductIdentifiersoLinkManager::_filterCompositePortscan dedup a board's secondary CDC port.WRITER_JOIN_TIMEOUT_MS) soclose()can't outlast the C++ disconnect timeout.Autoconnect latency (Android)
SerialPlatformgains asetDevicesChangedCallbackseam (host no-op).Host error plumbing
HostSerialPortroutes all errors through a single_setError()(cached code +errorString+errorOccurred), covering synthetic errors (invalid config, presence-loss, open failure) thatQSerialPortdoesn't report.mapHostErrornow maps the fullQSerialPort::SerialPortErrorset (OpenFailed/Read/Write/UnsupportedOperation/NotOpen) so the common open-failure case surfaces a meaningful code instead ofUnknown.Test
HostSerialPortTest,NmeaSerialDeviceTest(host/ctest); JUnit suites for the Java FSM, bridge, enumerator parsing, and D2xx library.