Repository navigation
Improve USB recovery, projection stability, and legacy Honda UI - #53
Merged
Merged
Conversation
Wrap negotiateAoap/requestAOAP in try/catch so USB exceptions during Start are reported through ConnectionManager instead of killing the process, and re-enable the global uncaught exception handler so any remaining crash is logged.
The custom UncaughtExceptionHandler installed in ODAApplication replaced Android's default handler and killed the process directly, so any crash on Start closed the app with zero log output and no crash dialog. Now it always logs via android.util.Log, cleans up defensively, and chains to the previous default handler so the OS crash dialog and FATAL EXCEPTION logcat entry are restored. Also adds a native signal handler (SIGSEGV/SIGABRT/SIGBUS/SIGILL/SIGFPE) in libcommon so a native/JNI crash writes a marker file before the process dies, since those bypass Java's exception handling entirely.
…og dir default PlayerActivity.onResume() called NotificationFactory.instance().dismissAll() right after the async startService(ODAService), before ODAService.onCreate() (which initializes NotificationFactory) had a chance to run, causing a NullPointerException on app start. Also correct the default external log directory to /mnt/sdcard/usbdrive2/ (no /ODA/ subfolder). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Three related use-after-free / data races found via device logcat during repeated connect/disconnect/mode-switch cycles: - LibUsbEndpoint::cancelTransfers() walked the transfers_ map from the caller's thread while transferHandler() (running on strand_) concurrently erased entries and freed the same libusb_transfer, causing SEGV inside libusb_cancel_transfer (seen from USBTransport::stop()). Now dispatched onto strand_ and awaited synchronously so callers can still safely destroy the endpoint right after it returns. - LibUsbDevice::releaseInterface()/claimInterface()/resetDevice() called the corresponding libusb_* functions without checking that handle_ was still valid, crashing in libusb_release_interface when AOAPDevice's destructor ran after the device handle had already been closed. Now guarded with a null check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Only :common:build and assembleRelease are needed to produce and sign the release APK; running the JVM unit tests and compiling the instrumented test APK on every push was pure overhead for that goal. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…fers reliably Three interlocking defects behind the SEGV in boost::asio's strand_service that kept killing the app during connect/disconnect cycles: - stopIOServiceWorkers() called detach() before testing joinable(), so the subsequent join() never ran: it returned while workers were still inside ioService_.run(), and the caller went straight on to destroy the messenger, cryptor and transport those threads were using. Joins now really happen. The calling thread is detached instead of joined, because teardown is also reached from a native quit callback running on one of these very workers, where a self-join would deadlock. - LibUsbEndpoint::cancelTransfers() was posting the cancels onto the strand. That raced with the io_service being stopped moments later and frequently meant no cancel was ever issued, leaving transfers submitted against a device handle that was then closed. It is synchronous again, but guarded by a mutex instead of the strand, so it neither depends on nor blocks the io_service. - transferHandler() no longer hops onto the strand for its own bookkeeping. Promise::resolve/reject already post to the strand they were deferred on, so running on libusb's thread is safe - and it means the entry is always erased and the transfer always freed, even after the io_service has stopped. Also drops the resubmit-on-error path: re-submitting a bulk transfer that already moved part of its buffer duplicates those bytes on the wire and desynchronises the AA framing (and its SSL layer). Upstream never had it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The logs showed a new session starting ~400ms before each native SEGV, with two AndroidAutoEntity alive over the same USB device. Causes, all fixed here: - ODAService.onDestroy() did nothing, so stopService() from the home screen or the exit widget destroyed the service with the session, its io_service threads and the USB handle still live. The replacement service then started with androidAutoEntity_ == null and built a second session on top. It now stops. - stop() was reentrant and could interleave with itself and with a start; a stopping_ flag serialises it. A flag rather than a lock because stop() arrives both from the UI thread and from native callbacks. - androidAutoEntity_ is volatile: it now gates whether a session may start, so a stale read could either skip a teardown or lock connecting out for good. - Anything thrown during teardown used to leave androidAutoEntity_ set, which made canStartSession() refuse every later attempt until a force stop. Each step is guarded and the field is cleared in finally. - HondaConnectManager.instance() can be null when the integration was enabled after boot; restores the null-checked accessor this fork had dropped, and guards the same call in stop(). Also: AppBadge.show() was starting an Intent aimed at AppBadge itself, which is not a Service and not in the manifest - the "Unable to start service it.smg.hu/<obfuscated>: not found" line in every log came from here. PlayerActivity now tracks the binding from bindService() rather than onServiceConnected (an onPause before the connection landed leaked it), keeps it bound across onServiceDisconnected (the binding survives the service dying), and null-guards AppBadge in onPause the way onResume already did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Android 4.0.4 is API 15, and DeviceFactory picks the video output with SDK_INT < JELLY_BEAN (16). So this head unit runs OMXVideoOutput/OMXVideoCodec - yet the volatile-flag hardening this fork added earlier went to NativeVideoOutput, which never executes here. The unprotected path was the one in use: - OMXVideoOutput: configured_/running_/videoCodec_ are volatile and write() reads the codec into a local, so a frame in flight can no longer pass the guard and then decode into a codec stop() has already nulled. - OMXVideoCodec: mediaDecode() and shutdown() are serialised, and both bail out once handle_ is 0. shutdown() used to nativeDelete() and only afterwards clear the handle, so a frame arriving in between was a use-after-free. - OMXVideoCodec.isSps() read buf.get(4) without a length check; a runt frame threw IndexOutOfBounds straight into the native caller, killing the thread that delivered it. - AudioCodec: the AudioTrack was a field, so a playback thread outliving its bounded join(1000) could release the track a freshly started thread was already writing to. It is now owned by the thread that created it, and a superseded thread notices via its own identity and exits. Also fixes an NPE on codecThread_.getName() after stop() nulled it - reachable whenever verbose logging is on, which is exactly how these logs were captured. - NativeVideoOutput: same volatile/local-read treatment for consistency, plus its output loop no longer spins at 100% CPU while unconfigured, and the input-buffer wait drops from 3s to ~2 frames (it runs on the transport thread, so a long wait stalls audio and control messages too). - DayNightSensor: quit() instead of interrupt(), which does nothing to a thread parked in MessageQueue.next() and leaked a HandlerThread per session; and startTimeCheck() reads the handler once instead of NPEing when teardown nulls it mid-callback. currentState_ is volatile here and in LightSensor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…age sticks Two things that made the app look far worse than it was on this head unit. ODALog spawned a "logcat -f" per app launch and never reaped the previous one. Being a separate process, it outlives the app being killed or crashing, so they accumulated: the ten logs from one debugging session were all still being written at the same second, pushing up to 2.1 MB/s onto the USB volume. Video frame rate tracked the count exactly - ~30fps with two of them, 0-2fps with eight - and audio was receiving under half the buffers it needed. It now records the child's pid (via "exec" in a shell, so the pid stays valid) and kills a leftover on the next launch, after confirming through /proc that the pid really is a logcat and not a recycled one. Note the filter is deliberately still "it.smg.hu:V" with no "*:S": a logcat filterspec matches exact tags, and everything here logs under "ODA/<subtag>" tags, so silencing the rest would empty the file entirely - and would also drop the DEBUG/ActivityManager/AudioFlinger lines that make native crashes and ANRs diagnosable. The shutdown pause also drops from 1500ms to 200ms, since it runs on the caller's thread including the crash handler. Separately, every USB attach and recovery pass tried the AOAP GET_PROTOCOL request on whatever was plugged in - including the head unit's own log/media stick (VID 0x090C), which can only ever fail. Eleven such failures were being surfaced as connection errors, drowning out the real state of the link. Negotiation now skips devices that expose only mass storage, hub or printer interfaces. That list is deliberately short: a phone looks vendor-specific over MTP, still-image over PTP and CDC/RNDIS while tethering, so excluding more would risk ignoring a real phone. Covered by tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Skipping the tests to speed up APK builds left them running nowhere at all, including newly added ones, and the API-level lint checks that matter for an API 15 target were never executed either. This project cannot be built locally without a JDK 17, so CI is the only place these run. Putting them in their own job keeps the APK turnaround unchanged while making the results real, and uploads the test/lint reports so failures are readable without re-running anything. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The unit tests are a real gate and they pass. Lint is not: a minSdk 15 project built against a modern compileSdk starts with a large pre-existing backlog, so failing every push on it only teaches people to ignore the run. The report is still uploaded as an artifact for triage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The first run of the check job failed at lint without producing a report, which left nothing to work from. Tee the task output and collect the intermediate lint XML too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Piping gradle into tee made the step exit with tee's status, so the previous run reported the lint step as successful while the task itself had failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Lint never ran a single check: it failed in :aasdk:configureNdkBuildDebug with "prefab/common/Android.mk: LOCAL_SRC_FILES points to a missing file". The app depends on aasdk, which consumes common through prefab, so lint drags in the whole native build - and this job had neither the NDK, nor libtinfo5, nor common's native output built first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
checkDependencies defaults to false, so :app:lintDebug was only looking at this module. The aasdk, common and omxvideocodec sources - the entire JNI Java layer - went unchecked, which is the last place you want an unguarded post-API-15 call on a minSdk 15 head unit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
|
This is a huge PR, thanks for the effort. I'll check it |
Owner
|
I’ve merged this pr into the development branch and am currently testing it. However, I noticed that you removed the USB_DEVICE_ATTACHED intent-filter and the corresponding handling in MainActivity (which was used to show a popup to launch ODA when a new smartphone is connected). Is there a specific reason for this change? Did you simply not need it anymore, or was there another reason? |
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
2 participants
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
This contribution updates OpenDroidAuto for legacy Android head units, especially Honda Connect systems. It focuses on USB connection and recovery, projection stability, and a more usable interface on older displays.
Motivation and validation
I am a Brazilian QA engineer and use this project in my own car with a legacy Honda multimedia unit. The changes came from issues I encountered in daily use, particularly USB synchronization and recovery. I developed and refined the Java and native code with AI assistance, then used the updated build in my car for roughly one to two months with good results.
The fork's Android CI completed successfully on the current head commit (
48371f83). Hardware validation is based on my Honda unit; I have not verified every other compatible head unit.I would be glad to work with maintainers on review feedback or split the changes if that makes integration easier.