Repository navigation
Conversation
…al app PixelPilot's own WfbngLink::initAgg() already sends decrypted video (RTP/H264) and MAVLink to 127.0.0.1:5600/14550 unconditionally via wfb-ng's AggregatorUDPv4 -- but the app's own in-app video player and MAVLink OSD parser also bind those same ports, so nothing else on the device can. This adds a way to free them up for any other UDP-based ground station app (QGroundControl, Mission Planner, a custom OSD, ...) running on the same device, without touching devourer/wfb-ng at all. Two independent settings-menu toggles under "Ground Station Streaming", "Video" and "Telemetry" (a ground station app might only care about one channel -- e.g. a video-only OSD box, or a telemetry-only companion computer): - Video: stops/starts the in-app UDPReceiver + decoder on port 5600. - Telemetry: stops/starts the in-app MAVLink OSD parser on port 14550. Both apply immediately (no restart) by starting/stopping the relevant in-app consumer right away, and persist via SharedPreferences so they survive a restart. onPause()/onStop() now skip wfbLinkManager.stopAdapters() while either channel is streaming, since the whole point is for wfb-ng to keep forwarding while this Activity is backgrounded (e.g. the ground station app in the foreground instead) -- both channels share one USB/wfb-ng adapter, so it can't be paused for one channel while kept alive for the other. This only covers ordinary Activity lifecycle transitions, not memory-pressure eviction; a Foreground Service would be needed for guaranteed long-running background survival, which is out of scope here. Verified on-device (RTL8812AU, Galaxy Tab S7 FE) with QGroundControl: both video and telemetry arrive correctly with their respective toggle enabled; each toggle independently frees/rebinds only its own port; the USB/wfb-ng pipeline is never restarted by toggling or by backgrounding while streaming is active; background/foreground cycling with only one channel enabled still keeps the pipeline alive. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PR Summary by QodoAdd independent video and telemetry ground station streaming toggles
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Code Review by Qodo
1. External apps cannot bind the telemetry port
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
| MavlinkNative.nativeStop(this); | ||
| handler.removeCallbacks(runnable); | ||
| } else { | ||
| MavlinkNative.nativeStart(this); | ||
| handler.post(runnable); |
There was a problem hiding this comment.
1. Telemetry never returns in-app 🐞 Bug ≡ Correctness
setTelemetryStreamingEnabled() calls nativeStop() and later nativeStart(), but the native stop signal is only incremented and is never reset before a replacement listener starts. Every enable-then-disable cycle therefore launches a listener that exits immediately, while a rapid cycle can additionally bind before the old socket releases port 14550.
Agent Prompt
## Issue description
Make the native MAVLink listener safely reusable so disabling ground-station telemetry reliably restores the in-app telemetry display.
## Issue Context
The new toggle stops and restarts the listener, but `nativeStop()` permanently increments a process-global signal and does not wait for the existing socket thread to finish. `nativeStart()` neither resets that signal nor synchronizes against the previous listener.
## Fix Focus Areas
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[1236-1244]
- app/mavlink/src/main/cpp/mavlink.cpp[57-58]
- app/mavlink/src/main/cpp/mavlink.cpp[377-388]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (!isGroundStationStreamingActive()) { | ||
| wfbLinkManager.stopAdapters(); | ||
| } |
There was a problem hiding this comment.
2. Exiting leaves the radio pipeline running 🐞 Bug ☼ Reliability
The new lifecycle conditions skip stopAdapters() whenever either persisted streaming preference is enabled, but the activity has no destruction cleanup for the retained native link, USB connection, thread, or callback timer. Finishing or recreating the activity therefore leaves its old pipeline holding the adapter and activity context, and reopening constructs another link that attempts to use the same statically tracked device.
Agent Prompt
## Issue description
Preserve streaming during temporary background transitions without orphaning the USB and native pipeline when the activity is actually destroyed.
## Issue Context
The activity now skips adapter shutdown in both pause and stop, while `WfbNgLink` owns per-instance native state, threads, USB connections, and a timer retaining the activity. Add explicit terminal cleanup or move ownership into a lifecycle-appropriate service; distinguish ordinary backgrounding from finishing or recreation.
## Fix Focus Areas
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[1660-1662]
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[1676-1680]
- app/wfbngrtl8812/src/main/java/com/openipc/wfbngrtl8812/WfbNgLink.java[53-67]
- app/src/main/java/com/openipc/pixelpilot/WfbLinkManager.java[209-233]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| private void setTelemetryStreamingEnabled(boolean enabled) { | ||
| getSharedPreferences("general", MODE_PRIVATE).edit().putBoolean(PREF_STREAM_TELEMETRY, enabled).apply(); | ||
| if (enabled) { | ||
| MavlinkNative.nativeStop(this); |
There was a problem hiding this comment.
1. External apps cannot bind the telemetry port 🐞 Bug ≡ Correctness
setTelemetryStreamingEnabled(true) calls nativeStop(), which signals the MAVLink listener to exit but never closes its socket bound to port 14550. When telemetry was already running in-app, that socket remains open after the listener exits, so the ground station cannot bind the port the toggle is meant to free.
Agent Prompt
## Issue description
Enabling telemetry streaming stops the listener thread but leaves its UDP socket bound, preventing another app from binding port 14550.
## Fix Focus Areas
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[1236-1244]
- app/mavlink/src/main/cpp/mavlink.cpp[52-101]
- app/mavlink/src/main/cpp/mavlink.cpp[318-325]
## Recommended Fix
Ensure the native listener closes its socket on every exit path, and complete listener shutdown and socket release before treating the telemetry handoff as complete.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (enabled) { | ||
| videoPlayer.stop(); | ||
| videoPlayer.stopAudio(); |
There was a problem hiding this comment.
4. Video handoff leaves recordings stalled 🐞 Bug ≡ Correctness
setVideoStreamingEnabled(true) stops the receivers without stopping an active DVR session or changing its recording indicator. If the user switches to ground-station video during a recording, the DVR receives no more decoded frames while its timer and recording UI continue until they manually stop it.
Agent Prompt
## Issue description
Switching video to an external ground station stops the source of DVR frames but leaves an active recording and its UI running.
## Fix Focus Areas
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[1220-1229]
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[1448-1504]
## Recommended Fix
Finalize any active DVR session before stopping the video receiver, and prevent starting an in-app DVR session while its video source is disabled.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if (enabled) { | ||
| videoPlayer.stop(); | ||
| videoPlayer.stopAudio(); |
There was a problem hiding this comment.
5. Video handoff keeps stale detection running 🐞 Bug ➹ Performance
setVideoStreamingEnabled(true) stops video reception without stopping the object-detection loop or hiding its overlay. With detection enabled, that loop continues sampling the video view and running inference even though the in-app video source has been disabled.
Agent Prompt
## Issue description
Object detection continues consuming resources after video streaming disables the in-app video receiver.
## Fix Focus Areas
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[1220-1229]
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[2181-2226]
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[2229-2317]
## Recommended Fix
Stop and clear the detection loop when video streaming is enabled; restart it when in-app video is restored if detection remains configured.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| private void setVideoStreamingEnabled(boolean enabled) { | ||
| getSharedPreferences("general", MODE_PRIVATE).edit().putBoolean(PREF_STREAM_VIDEO, enabled).apply(); | ||
| if (enabled) { | ||
| videoPlayer.stop(); |
There was a problem hiding this comment.
6. Video handoff can freeze the settings menu 🐞 Bug ☼ Reliability
setVideoStreamingEnabled(true) calls videoPlayer.stop() directly from the menu callback, and native stop synchronously joins both receiver threads. If either receiver is still processing a packet or has not exited its receive loop, the main thread waits for it before the menu interaction can finish.
Agent Prompt
## Issue description
The video-streaming menu action joins native receiver threads on the Android main thread.
## Fix Focus Areas
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[1220-1229]
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[1253-1261]
- app/videonative/src/main/cpp/VideoPlayer.cpp[190-204]
## Recommended Fix
Perform potentially blocking receiver teardown on a worker thread, and update the menu state on the main thread once the receiver has stopped and released its port.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Code review by qodo was updated up to the latest commit 24d53e9 |
josephnef
left a comment
josephnef
left a comment
There was a problem hiding this comment.
The use case is legitimate and the approach (free the in-app consumers so another app can bind 5600 / 14550, keep the adapter alive across pause) is the right shape. CI is green on the reopened run. A few things need to change before it can merge:
-
It depends on #124.
setTelemetryStreamingEnabled(false)callsMavlinkNative.nativeStartafter anativeStop, but on mastermavlink_thread_signalis never reset and the old listener never closes its socket, so telemetry never comes back in-app once toggled off. Qodo's first finding is correct. Merge #124 first and rebase on it. -
Finishing the activity leaks the adapter. With either toggle on,
onPauseandonStopskipstopAdapters(), and there is noonDestroyorisFinishing()path that stops it. Back-swiping out of the app leaves the USB adapter claimed and the wfb-ng threads running until the process dies, and the next launch has to recover from that. Stop the adapters whenisFinishing()is true (or inonDestroy), and keep the skip only for the pause/stop that is not a finish. -
In-app telemetry dies after the first background anyway.
onStopstill callsnativeStopunconditionally and nothing restarts the listener inonResume, so with the telemetry toggle off the OSD loses telemetry after the first pause. That is a pre-existing master bug, but this PR now owns the mavlink start/stop logic, so it should restart inonResumeunder the sameisTelemetryStreamingEnabled()gate it adds tosetupMavlink(). -
The comment in
onPausementions a devourer teardown race inRtlJaguarDevice. The submodule has moved a lot since (bb03774 now); if that was the motivation for skipping the stop, please recheck, and either way the comment should not describe a bug in another repo as a reason for app behaviour.
Documented limitation that is fine as-is: without a foreground service the process can still be killed in the background. Worth a line in the README section when you add one.
This branch has not been deployed
2 participants
PixelPilot's own WfbngLink::initAgg() already sends decrypted video
(RTP/H264) and MAVLink to 127.0.0.1:5600/14550 unconditionally via
wfb-ng's AggregatorUDPv4 -- but the app's own in-app video player and
MAVLink OSD parser also bind those same ports, so nothing else on the
device can. This adds a way to free them up for any other UDP-based
ground station app (QGroundControl, Mission Planner, a custom OSD, ...)
running on the same device, without touching devourer/wfb-ng at all.
Two independent settings-menu toggles under "Ground Station Streaming",
"Video" and "Telemetry" (a ground station app might only care about
one channel -- e.g. a video-only OSD box, or a telemetry-only
companion computer):
Both apply immediately (no restart) by starting/stopping the relevant
in-app consumer right away, and persist via SharedPreferences so they
survive a restart.
onPause()/onStop() now skip wfbLinkManager.stopAdapters() while either
channel is streaming, since the whole point is for wfb-ng to keep
forwarding while this Activity is backgrounded (e.g. the ground station
app in the foreground instead) -- both channels share one USB/wfb-ng
adapter, so it can't be paused for one channel while kept alive for the
other. This only covers ordinary Activity lifecycle transitions, not
memory-pressure eviction; a Foreground Service would be needed for
guaranteed long-running background survival, which is out of scope
here.
Verified on-device (RTL8812AU, Galaxy Tab S7 FE) with QGroundControl:
both video and telemetry arrive correctly with their respective toggle
enabled; each toggle independently frees/rebinds only its own port;
the USB/wfb-ng pipeline is never restarted by toggling or by
backgrounding while streaming is active; background/foreground cycling
with only one channel enabled still keeps the pipeline alive.
🤖 Generated with Claude Code