[dawitt50]

Adds a second link mode next to WFB-NG: the USB adapter (devourer userspace driver) joins the open Wi-Fi access point of an OpenIPC apfpv camera as a regular station, and the camera pushes its RTP stream to the phone's static IP.

• StaLink (native): probe/auth/assoc state machine, ARP, ICMP echo, a minimal TCP client for the camera's air_man_ap control port, UDP forwarding to the local player (5600) and MAVLink (14550).
• Connects without a scan (last known channel, then the apfpv boot channels; full sweep optional).
• Follows channel switch announcements, including 40/80 MHz width changes.
• Dropouts don't end the association; rejoin goes straight to the known BSSID after a deauth or a camera restart (beacon TSF jump). A key frame is requested whenever video can flow again.
• App: Link mode setting, APFPV settings dialog, Channel menu sends set_ap_channel, and Drone → Settings opens native camera controls (air_man_ap + aalinkFPV web API).
• README: new section "Link modes: WFB-NG and APFPV".

Camera side (apfpv firmware)

The stock firmware runs a WPA2 access point with a DHCP server. For this mode, /usr/bin/adapter (which writes hostapd.conf on every start) was patched:

• WPA2 off: wpa_passphrase and rsn_pairwise removed, wpa=2 -> wpa=0 (open network, no 4-way handshake when joining).
• No DHCP: udhcpd is no longer started, nor restarted when a station connects; the ground station uses the fixed IP 192.168.0.10. Instead of the DHCP reset, only the station's ARP entry is cleared (ip neigh del).
• disassoc_low_ack=0: hostapd no longer drops the station when many frames go unacknowledged during a fade, so the association survives a dropout and video resumes without rejoining.
• Optional MAC filter (macaddr_acl=1 with an accept_mac_file) so only the ground station's adapter MAC can join.
• majestic sends the stream to the ground station (outgoing.server: udp://192.168.0.10:5600).

The camera changes are scripted in tools/apfpv-open-static-cam.sh (keeps a backup, --revert restores it).

Limitations

Open networks only (no WEP/WPA), static IP (no DHCP), no 802.11 fragment reassembly, UDP only plus one short TCP request at a time.

Tested on a RTL8812EU adapter with OpenIPC apfpv firmware.

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Add APFPV station link mode for OpenIPC cameras

✨ Enhancement 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Add APFPV mode so a USB adapter can join an OpenIPC camera’s open access point.
• Forward camera video and MAVLink locally, with channel-switch and dropout recovery.
• Add camera controls, a reversible camera setup script, and configuration guidance.

Diagram

graph TD
  UI["Video Activity"] --> Bridge["Java link API"] --> Native["Native link"] --> Station["StaLink stack"] --> Adapter["USB adapter"] --> Camera["Camera AP"]
  UI --> Controls["Camera controls"] --> Bridge
  Camera --> Station --> Player["Local player"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use an established embedded TCP/IP stack

• ➕ Reduces the amount of custom transport-protocol logic to maintain and validate.
• ➖ Adds a dependency and requires integration with devourer’s raw-frame and station paths.

2. Use the phone’s standard Wi-Fi connection

• ➕ Avoids implementing a station and IP stack in the app.
• ➖ Does not meet the goal of joining through the USB adapter while preserving the phone’s own networking.

Recommendation: The devourer-based station path fits the USB-adapter requirement and reuses the existing local video and MAVLink consumers. Keep the protocol scope narrow, but prioritize hardware and packet-level testing of association, channel switches, fragmentation, and TCP recovery; an established TCP/IP stack is worth considering if control traffic expands.

Files changed (11) +3605 / -25

Enhancement (8) +3402 / -25

CameraControlDialog.javaAdd native APFPV camera controls +737/-0

Add native APFPV camera controls

• Adds a settings dialog for air_man_ap commands and authenticated aalinkFPV HTTP endpoints. Serializes requests on a background executor and supports transport over either the USB adapter or the phone’s Wi-Fi.

app/src/main/java/com/openipc/pixelpilot/CameraControlDialog.java

VideoActivity.javaExpose APFPV mode, settings, and recovery status +474/-7

Expose APFPV mode, settings, and recovery status

• Persists link and camera settings, adds mode-aware menus and camera controls, and switches adapter and VPN behavior with the selected mode. Displays AP connection statistics, remembers the camera channel, and requests a key frame after recovery; also makes the WFB-NG drone web address configurable.

app/src/main/java/com/openipc/pixelpilot/VideoActivity.java

WfbLinkManager.javaShow APFPV-specific adapter startup status +7/-1

Show APFPV-specific adapter startup status

• Tracks whether APFPV mode is selected for startup messaging and shows a camera-join message instead of the WFB-NG channel message.

app/src/main/java/com/openipc/pixelpilot/WfbLinkManager.java

StaLink.cppImplement the APFPV userspace station and packet path +1551/-0

Implement the APFPV userspace station and packet path

• Implements open-network discovery and association, channel and width tracking, dropout recovery, and adapter frame handling. Processes ARP, ICMP, IPv4 fragments, incoming UDP forwarding, and one short TCP request at a time for camera control.

app/wfbngrtl8812/src/main/cpp/StaLink.cpp

StaLink.hDefine station configuration, state, and transport interfaces +262/-0

Define station configuration, state, and transport interfaces

• Declares the station lifecycle, connection and recovery status, traffic counters, and blocking camera TCP request API. Defines the configuration and internal state used for scanning, retuning, packet handling, and transport sessions.

app/wfbngrtl8812/src/main/cpp/StaLink.h

WfbngLink.cppWire APFPV station mode into the native link +267/-17

Wire APFPV station mode into the native link

• Branches adapter startup into StaLink, configures station transmission and ACK behavior, and releases devices on shutdown. Adds JNI entry points for AP configuration, status, recovery counts, and camera commands while retaining the WFB-NG path.

app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp

WfbngLink.hppAdd native link-mode and station ownership state +34/-0

Add native link-mode and station ownership state

• Defines WFB-NG and APFPV modes, stores station configuration and its shared lifetime, and declares station startup and device-release helpers.

app/wfbngrtl8812/src/main/cpp/WfbngLink.hpp

WfbNgLink.javaExpose APFPV operations to Android +70/-0

Expose APFPV operations to Android

• Adds Java mode constants, native declarations, and wrappers for station settings, connection status, recovery counts, and camera TCP commands.

app/wfbngrtl8812/src/main/java/com/openipc/wfbngrtl8812/WfbNgLink.java

Documentation (1) +77 / -0

README.mdDocument WFB-NG and APFPV link modes +77/-0

Document WFB-NG and APFPV link modes

• Explains APFPV setup, channel discovery and switching, camera controls, dropout behavior, and protocol limitations. Includes the camera-side setup command and majestic stream destination.

README.md

Other (2) +126 / -0

CMakeLists.txtBuild the new station implementation +2/-0

Build the new station implementation

• Adds StaLink source and header files to the native shared-library target.

app/wfbngrtl8812/src/main/cpp/CMakeLists.txt

apfpv-open-static-cam.shProvide reversible OpenIPC AP preparation +124/-0

Provide reversible OpenIPC AP preparation

• Backs up and patches the camera’s adapter script to run an open AP without DHCP and to retain associations through fades. Supports an optional MAC allowlist, restart control, and restoration from the backup.

tools/apfpv-open-static-cam.sh

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. A rejected adapter can crash later controls ✓ Resolved

Description

runStation() assigns current_fd before checking whether another adapter already owns the AP
association. If a second adapter is rejected, its device is erased but its descriptor remains
current after the first adapter exits, so a later adaptive-link or transmit-power operation reaches
rtl_devices.at() with a missing descriptor.

Code

app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[306]

+    current_fd = fd;

Evidence

The second run overwrites the descriptor before rejection and erases that device; cleanup of the
first device cannot clear the overwritten value, while AP-mode controls dereference it.

app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[304-331]
app/wfbngrtl8812/src/main/cpp/WfbngLink.hpp[146-149]
app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[675-680]
app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[806-826]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A rejected second AP-mode adapter overwrites the active descriptor, leaving later device controls able to access an erased device.
## Fix Focus Areas
- app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[304-331]
- app/wfbngrtl8812/src/main/cpp/WfbngLink.hpp[146-149]
## Recommended Fix
Set `current_fd` only after accepting the adapter as the station, and ensure rejection cannot change the active adapter's descriptor.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Video fragments can assemble with gaps ✓ Resolved

Description

reassemble() counts every fragment's length toward completion without checking which byte ranges
have arrived. An overlapping fragment or a delayed duplicate can make the count reach the expected
size while bytes are missing, after which the zero-filled datagram can be forwarded to the video
listener.

Code

app/wfbngrtl8812/src/main/cpp/StaLink.cpp[1036]

+    it->received += plen;

Evidence

The implementation copies at each offset but increments a plain byte sum; its completion check then
dispatches the reconstructed packet to the UDP forwarding path. Retry suppression compares only with
the immediately previous sequence number, so it does not rule out a delayed duplicate.

app/wfbngrtl8812/src/main/cpp/StaLink.cpp[1030-1047]
app/wfbngrtl8812/src/main/cpp/StaLink.cpp[964-968]
app/wfbngrtl8812/src/main/cpp/StaLink.cpp[1093-1101]
app/wfbngrtl8812/src/main/cpp/StaLink.cpp[1113-1152]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Summing fragment lengths treats overlapping or repeated bytes as newly received bytes and can complete an IP datagram with a gap.
## Fix Focus Areas
- app/wfbngrtl8812/src/main/cpp/StaLink.cpp[1009-1047]
- app/wfbngrtl8812/src/main/cpp/StaLink.h[224-236]
## Recommended Fix
Track received byte ranges or blocks per datagram, and return a reassembled packet only when every byte through the final fragment has been received.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. A missing MAC argument drops the filter ✓ Resolved

Description

The --allow-mac parser shifts to an argument without first checking that one exists. When the
option is last, MAC stays empty and the script continues past the failed shift, skips both
validation and ACL installation, and installs the open-network configuration.

Code

tools/apfpv-open-static-cam.sh[R45-48]

+        --allow-mac) shift; MAC=$(echo "$1" | tr 'A-F' 'a-f') ;;
+        *) die "unknown option: $1" ;;
+    esac
+    shift

Evidence

Argument handling has no missing-value check, and both validation and ACL insertion are conditional
on a nonempty MAC.

tools/apfpv-open-static-cam.sh[41-49]
tools/apfpv-open-static-cam.sh[68-70]
tools/apfpv-open-static-cam.sh[99-105]
tools/apfpv-open-static-cam.sh[118-120]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Invoking `--allow-mac` without its value silently installs without the requested MAC restriction.
## Fix Focus Areas
- tools/apfpv-open-static-cam.sh[41-49]
- tools/apfpv-open-static-cam.sh[68-70]
## Recommended Fix
Check that an argument remains immediately after `--allow-mac` and exit with an error before modifying the adapter if it does not.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. An ACL write failure leaves a partial install ✓ Resolved

Description

The script replaces /usr/bin/adapter before writing the requested MAC accept file. If that write
fails, it exits without restoring the adapter, leaving the open-network configuration installed with
a reference to a missing or unwritten ACL file.

Code

tools/apfpv-open-static-cam.sh[R118-119]

+mv "$TMP" "$ADAPTER" && chmod 755 "$ADAPTER" || die "writing $ADAPTER failed"
+[ -n "$MAC" ] && { echo "$MAC" > "$ACCEPT" || die "writing $ACCEPT failed"; echo "Only $MAC may join ($ACCEPT)."; }

Evidence

ACL directives are inserted into the generated adapter, which is moved into place before the
accept-file write that can call die.

tools/apfpv-open-static-cam.sh[99-109]
tools/apfpv-open-static-cam.sh[114-120]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Failure to create the MAC accept file leaves the adapter replaced despite the installation command failing.
## Fix Focus Areas
- tools/apfpv-open-static-cam.sh[96-120]
## Recommended Fix
Create and verify the accept file before replacing the adapter, or restore the backed-up adapter and clean up on any later failure.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

5. Brief fades miss the key-frame request ✓ Resolved

Description

tick() is the only place that sets in_gap_, while onRx() increments the recovery counter only
if that flag is already set. If reception resumes just after the 250 ms dropout threshold but before
the next worker tick, the app receives no recovery change and does not request a new key frame.

Code

app/wfbngrtl8812/src/main/cpp/StaLink.cpp[R688-692]

+        if (in_gap_ && state_.load() == State::Connected) {
+            // Dropout over: video flows again right now, nothing to rebuild.
+            in_gap_ = false;
+            recoveries_++;
+            __android_log_print(ANDROID_LOG_INFO,

Evidence

The worker checks for silence periodically, but reception checks only the flag; Java requests a key
frame solely on a changed recovery count.

app/wfbngrtl8812/src/main/cpp/StaLink.cpp[30-42]
app/wfbngrtl8812/src/main/cpp/StaLink.cpp[560-565]
app/wfbngrtl8812/src/main/cpp/StaLink.cpp[688-708]
app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[2167-2175]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A dropout that ends between worker ticks never sets the recovery flag, so the video refresh request is skipped.
## Fix Focus Areas
- app/wfbngrtl8812/src/main/cpp/StaLink.cpp[688-708]
- app/wfbngrtl8812/src/main/cpp/StaLink.cpp[560-565]
## Recommended Fix
When an AP packet arrives, compare its arrival time with `last_heard_` and apply the dropout threshold there as well, before deciding whether to increment recoveries.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

6. A fast restart can miss a key frame ✓ Resolved

Description

lastApRecoveries persists across adapter runs, but each new StaLink starts its recovery counter
at zero. If the new association reaches the previous count before the next periodic stats callback,
the equality check misses that association and skips its key-frame request.

Code

app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[R2170-2174]

+        if (recoveries != lastApRecoveries) {
+            lastApRecoveries = recoveries;
+            if (recoveries > 0) {
+                requestKeyFrame();
+            }

Evidence

The native counter is initialized per station instance, whereas the activity compares only its
numeric value on a timer independent of station creation.

app/wfbngrtl8812/src/main/cpp/StaLink.h[199-203]
app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[318-323]
app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[629-635]
app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[2124-2126]
app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[2167-2175]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new station can reuse the previous station's recovery count before stats observe an intermediate zero, hiding a new association.
## Fix Focus Areas
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[2124-2126]
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[2167-2175]
## Recommended Fix
Associate the observed count with an adapter generation, or reset the saved count when restarting adapters so the new association always triggers a key-frame request.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

View review recommended (5)

7. Delayed replies can truncate camera settings ✓ Resolved

Description

tcpTick() finishes a request after 1.5 seconds without new bytes even when the peer has not closed
and an HTTP response is incomplete. If a later response segment is delayed beyond that interval,
cgi() receives a non-null partial reply and returns on JSON parse failure rather than trying the
request again.

Code

app/wfbngrtl8812/src/main/cpp/StaLink.cpp[R1275-1277]

+    if (tcp_.st == St::Established && !tcp_.rx.empty() && now - tcp_.last_rx >= kTcpReplyIdle) {
+        tcpFinish(true);
+        return;

Evidence

The native idle path marks any nonempty reply complete; the Java parser retries null transport
replies but immediately returns for a non-null reply containing incomplete JSON.

app/wfbngrtl8812/src/main/cpp/StaLink.cpp[1243-1252]
app/wfbngrtl8812/src/main/cpp/StaLink.cpp[1270-1278]
app/src/main/java/com/openipc/pixelpilot/CameraControlDialog.java[518-541]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
An inter-segment delay makes native TCP return a partial but non-null HTTP reply, which the camera-settings parser does not retry.
## Fix Focus Areas
- app/wfbngrtl8812/src/main/cpp/StaLink.cpp[1270-1280]
- app/src/main/java/com/openipc/pixelpilot/CameraControlDialog.java[518-541]
## Recommended Fix
For HTTP requests, determine completion from the response framing or peer FIN rather than the short idle interval; also retry incomplete or unparseable replies within the existing request retry loop.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

8. Camera commands fail when another is in flight ✓ Resolved

Description

StaLink::tcpRequest returns false straight away when tcp_.st != St::Idle. Callers read that as
"no answer": requestKeyFrame and the AP Channel menu (onApChannelSelected) don't retry, so they
give up, and only the camera dialog's airman/cgi retry. A key-frame request started by a
recovery (for example during a channel switch or right after a rejoin) can therefore make a user's
set_ap_channel report "Camera did not answer, channel unchanged". In the other direction, a
running dialog or channel command silently drops the IDR request that recovery needed.

Code

app/wfbngrtl8812/src/main/cpp/StaLink.cpp[1208]

+    if (!running_ || state_.load() != State::Connected || tcp_.st != St::Idle) return false;

Evidence

There is only one TCP session slot. A busy slot returns false right away without waiting.
nativeCameraCommand passes that on as null, and the Java callers treat null as the camera not
answering. Only CameraControlDialog retries.

app/wfbngrtl8812/src/main/cpp/StaLink.cpp[1200-1210]
app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[2133-2154]
app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[487-498]
app/src/main/java/com/openipc/pixelpilot/CameraControlDialog.java[505-514]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`StaLink::tcpRequest` returns false right away when another TCP request is in flight. Callers can't tell this apart from "camera did not answer", so key-frame requests and AP channel switches fail at random.
## Fix Focus Areas
- app/wfbngrtl8812/src/main/cpp/StaLink.cpp[1200-1210]
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[2133-2154]
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[487-498]
## Recommended Fix
In `tcpRequest`, if `tcp_.st != Idle`, wait on `tcp_cv_` (with the caller's timeout) until the slot goes back to Idle before starting, and return false only when not connected or the wait times out. Have the callers notify waiters when they reset `tcp_.st = Idle`. Another option is to serialize requests with a dedicated mutex held for the whole request.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

9. Adapter object can be freed while stop runs ✓ Resolved

Description

The new releaseDevice(fd) call at the end of run()/runStation() erases the shared_ptr from
rtl_devices on the run thread. Meanwhile WfbngLink::stop() on the Java thread does find/at
on the same unsynchronized map and calls StopRxLoop() and stop_adaptive_link() through a raw
pointer. When stop() ends the RX loop, the run thread wakes up and can destroy the device (and
mutate the map) while stop() is still inside StopRxLoop(). That is a use-after-free and a data
race on std::map. Before this PR, entries were never erased on the normal exit path.

Code

app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[R374-375]

+    dev->Stop();
+    releaseDevice(fd);

Evidence

stop() takes a raw IRtlDevice* from rtl_devices without any lock. releaseDevice erases the
owning shared_ptr from the RX thread as soon as StartRxLoop returns. nativeSetTxPower also
reads the map through current_fd without a lock.

app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[382-398]
app/wfbngrtl8812/src/main/cpp/WfbngLink.hpp[142-149]
app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[365-379]
app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[810-819]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`releaseDevice()` now erases devices from `rtl_devices` at the end of `run()`. `stop()` and `nativeSetTxPower` read the same map and call methods through raw pointers on other threads, with no synchronization.
## Fix Focus Areas
- app/wfbngrtl8812/src/main/cpp/WfbngLink.hpp[142-149]
- app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[382-398]
- app/wfbngrtl8812/src/main/cpp/WfbngLink.cpp[810-819]
## Recommended Fix
Add a mutex around every `rtl_devices` access (insert, erase, find/at, and `current_fd`). In `stop()` and the TX power setter, copy the `shared_ptr<IRtlDevice>` while holding the lock and call through that copy, so the object stays alive until the call returns.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

10. Failed channel switch is taken as success ✓ Resolved

Description

StaLink::worker() catches exceptions from FastRetune() or SetMonitorChannel() but still sets
channel_, width_mhz_, chan_offset_, and dwell_start_ to the requested values and clears the
pending switch. If the driver rejects a CSA width change or scan hop, the radio remains on its old
settings while search and probe decisions, the status line, and connectedChannel() use the false
state, with no retune retry scheduled.

Code

app/wfbngrtl8812/src/main/cpp/StaLink.cpp[R297-304]

+            } catch (const std::exception &e) {
+                __android_log_print(ANDROID_LOG_ERROR, TAG, "retune to ch %d failed: %s", retune, e.what());
+            }
+            std::lock_guard<std::mutex> lock(mu_);
+            channel_ = static_cast<uint8_t>(retune);
+            width_mhz_ = retune_width;
+            chan_offset_ = retune_offset;
+            dwell_start_ = Clock::now();

Evidence

The retune calls can throw, but their catch block only logs the exception; the following
lock-guarded assignments update the tuned-channel and width state regardless of success. Subsequent
channel-search logic relies on those assignments, so a failed driver switch is treated as though it
completed.

app/wfbngrtl8812/src/main/cpp/StaLink.cpp[273-306]
app/wfbngrtl8812/src/main/cpp/StaLink.cpp[273-305]
app/wfbngrtl8812/src/main/cpp/StaLink.cpp[466-485]
app/wfbngrtl8812/src/main/cpp/StaLink.cpp[552-588]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
In `StaLink::worker()`, a failed retune is recorded as successful, leaving station channel state, search behavior, and reported channel inconsistent with the radio.
## Fix Focus Areas
- app/wfbngrtl8812/src/main/cpp/StaLink.cpp[273-306]
- app/wfbngrtl8812/src/main/cpp/StaLink.cpp[466-485]
## Recommended Fix
Track a success flag set only when `FastRetune()` or `SetMonitorChannel()` returns normally, and update the tuned channel, width, offset, and dwell bookkeeping only on success. On failure, retain the actual settings and arrange a retry or recovery search—for example, restore `retune_to_` under `mu_` or fall back to a 20 MHz `SetMonitorChannel` on the requested primary channel.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

11. Camera login is sent in clear over open Wi-Fi 🐞 Bug ⛨ Security

Description

requestKeyFrame and CameraControlDialog.cgi() send the drone username and password as HTTP Basic
auth over the camera's access point. With this PR (and tools/apfpv-open-static-cam.sh) that access
point has WPA2 removed. Anyone in radio range can capture those credentials, which are usually the
camera's root login, and replay them against the camera's web API. Key-frame requests go out
automatically on every association and every end of a dropout, so the credentials are broadcast
often, not only when the user opens the controls.

Code

app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[R2139-2144]

+        String auth = Base64.encodeToString((getDroneUsername() + ":" + getDronePassword())
+                .getBytes(java.nio.charset.StandardCharsets.UTF_8), Base64.NO_WRAP);
+        String request = "GET /request/idr HTTP/1.0\r\n"
+                + "Host: " + cameraIp + "\r\n"
+                + "Authorization: Basic " + auth + "\r\n"
+                + "Connection: close\r\n\r\n";

Evidence

The script turns WPA off (wpa=0, passphrase removed), and the MAC filter is optional. The app builds
a Basic Authorization header from the stored drone credentials and sends it in plaintext HTTP over
that open link.

app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[2133-2154]
app/src/main/java/com/openipc/pixelpilot/CameraControlDialog.java[523-530]
tools/apfpv-open-static-cam.sh[100-108]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
APFPV mode runs over an unencrypted access point, yet the app sends the drone login as HTTP Basic auth, automatically for every key-frame request and for every camera-control request.
## Fix Focus Areas
- app/src/main/java/com/openipc/pixelpilot/VideoActivity.java[2133-2154]
- app/src/main/java/com/openipc/pixelpilot/CameraControlDialog.java[523-530]
- README.md
## Recommended Fix
Request key frames through an unauthenticated channel (for example an air_man_ap command, or majestic's IDR request on a port with no credentials), or make the automatic key-frame request opt-in. Warn the user in the APFPV settings dialog and in the README that the camera password is sent in clear over the open network, and recommend a dedicated password for this mode.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Tip of the day

💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[josephnef]

Thanks for this, it is a substantial and well-structured piece of work. The station state machine, the ARP/ICMP/TCP mini-stack and the devourer integration are more careful than most contributed network code: every header parse I traced is length-checked, lock ordering is consistent (mu_ → q_mu_, sta_mutex → mu_ only from JNI), the six new JNI signatures match, and the existing R8 keep rules already cover the new natives. No merge conflicts with master.

I am requesting changes for two blocking items and a handful of robustness fixes. Qodo's review above is largely accurate; where I agree I reference its numbering rather than repeat it.

CI preview

CI (pr.yml, merged after this PR was opened) has not run here yet. It will on your next push. Expect:

• clang-format job: fail. With the pinned clang-format 18.1.8 the diff is 195 lines in StaLink.cpp, 22 in StaLink.h, 57 in WfbngLink.cpp, 4 in WfbngLink.hpp (mostly bin-packed arguments and one-line loop bodies, plus one 124-column line at StaLink.cpp:695). Fix with pip install clang-format==18.1.8 && scripts/format_sources.sh.
• Host GoogleTest and debug APK build: expected to pass. The emulator smoke job is non-blocking.

Blocking

1. Device teardown race, and it now affects WFB-NG mode too (Qodo #9, #1). releaseDevice(fd) is called at the end of run() / runStation() (WfbngLink.cpp:278, 296, 375) and erases from rtl_devices (WfbngLink.hpp:146-149). Master only erased on the early-abort path. rtl_devices is an unsynchronised std::map that stop() (find then at(fd).get() then StopRxLoop()), nativeSetTxPower (:817-818) and start_link_quality_thread (:679, 786) read from other threads. When the RX loop exits on its own (USB detach, libusb error) and WfbLinkManager then calls stop, the run thread can destroy the device while stop() is inside StopRxLoop() through a raw pointer, and .at() on an erased fd throws through JNI. Keep the erase if devourer's per-adapter lock needs it, but guard rtl_devices / current_fd with a mutex and have stop() and the setters take a shared_ptr copy under it. Also set current_fd only after the adapter is accepted as the station (Qodo #1).
2. Formatting, see above.

Should fix

• ICMP echo of arbitrary size is injected as one MPDU (StaLink.cpp:1161-1175). The reply copies the whole datagram, and reassembly allows up to 64 KB, so anyone on the open AP can make the worker hand a ~60 KB frame to send_packet. Drop echo requests above ~1500 bytes.
• Fragment reassembly completes with gaps (Qodo #2, StaLink.cpp:1036). received += plen counts overlapping or duplicate fragments, so a zero-filled datagram can be forwarded to :5600. Track coverage or drop overlaps.
• Unbounded buffers: tcp_.rx grows until the deadline with no cap (StaLink.cpp:1348, cap at e.g. 64 KB); seen_ grows per distinct BSSID (:836, cap it).
• SetTxPower from the JNI thread races the worker's SetMonitorChannel / FastRetune (StaLink.cpp:280-289). devourer's IRtlDevice.h:111-116 requires a single control thread. Queue it through the worker like requestTune.
• Qodo #10 (failed channel switch recorded as success, StaLink.cpp:304), #8 (TCP single-flight: a recovery key-frame request makes a user's set_ap_channel report "camera did not answer", :1208), #7 (partial HTTP reply accepted after 1.5 s idle, :1277), #5 and #6 (gap-recovery edge cases). All hold on my read.
• tools/apfpv-open-static-cam.sh: --allow-mac as the last argument silently installs without the ACL (Qodo #3, :48); the adapter is replaced before the ACL file is written, so a write failure leaves a half-installed config (Qodo #4, :119); and $master is assumed defined at the ip neigh del site (:107-108), add it to the pre-flight checks.

Document

• Open network without PMF: a spoofed deauth or a spoofed beacon carrying the SSID can steer the station (StaLink.cpp:754, 836). Inherent to the design, but the README should say so and recommend --allow-mac.
• Camera HTTP credentials go as Basic auth in cleartext over the open AP (VideoActivity.java:2140, CameraControlDialog.java:519). Worth one sentence.

What would make this mergeable

1. Run scripts/format_sources.sh with clang-format 18.1.8.
2. Mutex around rtl_devices / current_fd, shared_ptr copies in stop() and the setters.
3. Bound network input (ICMP echo size, tcp_.rx, seen_) and fix fragment coverage.
4. Serialise SetTxPower with the worker's retunes.
5. README notes on the security model.

If you want something merged sooner, the camera-side script plus its README section would stand on their own as a separate PR.

[josephnef]

Heads-up: #126 (background VPN start fix) and #124/#122 are merged, so this branch now conflicts in VideoActivity.java around onPause. Please rebase on current master together with the changes requested above; the onPause stop now goes through WfbNgVpnService.stopRunning().

[dawitt50]

Thanks for the thorough reviews, @josephnef and Qodo. I've rebased on current master and addressed the points below. The branch was force-pushed, so the head is now e5d34f7.

Rebase

• Rebased onto a68a61f. The VideoActivity conflict is resolved: the master side (startServiceIfAllowed) is kept, the PR's own STOP_SERVICE helper is gone, and switching to APFPV mode now stops the tunnel through WfbNgVpnService.stopRunning().

Blocking

• Device teardown race (Qodo Added support for Tenda U12 Wifi Adapter (RTL8812AU based) #1, Can not selecte a different key from phone #9): rtl_devices and current_fd are guarded by dev_mutex. stop() and the TX power setter call into the device only through withDevice(), which holds the lock for the call. StopRxLoop() only sets a flag, so this is cheap, and no other thread ever holds a reference. The device is therefore always destroyed on the run thread, before libusb_exit(). current_fd is atomic and is set in runStation() only once the adapter is accepted. A rejected second adapter no longer overwrites it, and releasing an adapter only clears it if it still points at that adapter.
• Formatting: clang-format 18.1.8 with scripts/format_sources.sh, as a separate commit (6138ce3). The check is clean.

Should fix

• ICMP echo size: echo requests over 1500 bytes are not answered.
• Fragment reassembly with gaps (Qodo Update usb_device_filter.xml #2): received 8-byte units are tracked per datagram. Duplicates and overlaps can no longer complete a datagram that still has holes, and a non-final fragment must be a multiple of 8 bytes.
• Unbounded buffers:
  • tcp_.rx is capped at 64 KiB. A reply over the cap ends the request immediately.
  • seen_ is capped at 64 entries; the stalest entry is evicted.
• SetTxPower vs. retunes: in AP mode the TX power is queued to the StaLink worker (requestTxPower()), the same path retunes take. Without a running station it is not applied from the JNI thread; runStation() applies it during bring-up.
• Failed retune taken as success (Qodo Add VR feature, Add record button #10): channel, width and offset are only updated when the switch succeeded. A refused switch is requested again up to 3 times unless a newer one is pending. Otherwise a refused switch to an announced channel would strand the station, because the dropout search only covers the boot channels.
• TCP single flight (Qodo Meta Quest native support #8): tcpRequest() waits for the session slot instead of failing. One deadline covers both the wait and the exchange.
• Partial HTTP replies (Qodo Add VR feature. Screen will be splitted to 2 when mode is On #7): HTTP requests (HTTP/1.0, Connection: close) now count as answered only when the camera closes the connection. The 1.5 s idle rule applies only to air_man_ap. A reset or a deadline during an HTTP reply is a failure, so cgi() retries. It also retries on an unreadable or missing JSON body.
  • Exception: actions that reboot the camera (reboot, hostapd start channel, SSID change) are sent only once. A retry there could reboot the camera a second time, and a missing answer is reported as "may already be rebooting".
• Brief fades miss the key-frame request (Qodo No choice of codec #5): onRx() also checks the silence against kDropoutGap, so a dropout that ends between two ticks is counted.
• Fast restart misses a key frame (Qodo No Video on MediaTek Device (Samsung Galaxy A15)? #6): the recovery counter the UI sees no longer restarts at zero with each new StaLink (ap_recovery_base).
• tools/apfpv-open-static-cam.sh (Qodo Add ability to send log files #3, Update usb_device_filter.xml #4, $master):
  • --allow-mac without a value aborts.
  • The accept file is written and read back before the adapter is replaced; if replacing fails, the original is restored.
  • The pre-flight check requires the adapter to define $master.

Documented

• The README has a security paragraph:
  • The access point is open and has no PMF, so traffic can be read and forged, and spoofed deauths and beacons can steer the station.
  • It recommends installing with --allow-mac and says what the filter does not protect against.
  • The Drone login goes out as cleartext Basic auth, including the automatic key-frame request after every (re)association and every end of a dropout.

Open: Qodo #11 (cleartext credentials)
This one is documented, not solved. Options I see:

1. Send the key-frame request without auth, if majestic allows /request/idr unauthenticated.
2. Make the automatic key-frame request opt-in.
3. Leave it as it is, with the README warning.

Which would you prefer?

Verification
assembleDebug builds (arm64-v8a, armeabi-v7a), and the clang-format check is clean. Tested on hardware with an RTL8812EU adapter against an apfpv camera, with no issues:

• join, video and channel switch from the menu
• dropout and recovery, including the key-frame request
• camera controls: loading values and changing a setting
• reboot from the dialog, with no second reboot
• unplugging and replugging the adapter (the teardown path from the blocking item)

The camera script was exercised against a mock adapter file: missing MAC value, unwritable accept file, missing $master, install with and without MAC, revert. The new StaLink paths have no unit tests. Not tested on RTL8812AU, the chip where devourer sends without hardware retries.

[josephnef]

Thanks for the fast and complete turnaround. I re-verified every item against e5d34f7 rather than taking the summary on trust, and all of it checks out:

• Teardown race: dev_mutex + withDevice() now cover every rtl_devices access (insert, stop(), applyTxPower(), releaseDevice()), no raw IRtlDevice* escapes the lock, the device is only ever destroyed on the run thread, and dev_mutex is never nested inside mu_, sta_mutex or thread_mutex or held across a join. current_fd is atomic and only published for the accepted adapter. This also removes a latent race in WFB-NG mode.
• ICMP cap at 1500, fragment coverage bitmap with the 8-byte rule, tcp_.rx and seen_ caps, TX power routed through the worker in AP mode and applied at bring-up otherwise, retune failure no longer recorded as success with bounded retries, single deadline on the TCP slot wait, HTTP completion on FIN with cgiOnce() for rebooting actions, the two gap-recovery edge cases, the script fixes, and the README security section: all confirmed in code.
• Rebase is clean: onPause is identical to master and the APFPV switch uses WfbNgVpnService.stopRunning().

CI on this head: clang-format, host GoogleTest, debug APK and the emulator smoke test are all green.

Key-frame request over cleartext Basic auth. My recommendation is option 1 with a fallback, as a follow-up PR rather than a blocker here: send GET /request/idr without Authorization first; on a 401, resend once with the credentials and remember that for the session. The TCP client already returns the status line, so this is a handful of lines in the key-frame path. That stops the recurring root-credential leak on every camera that does not gate the endpoint, costs one extra round-trip once on the ones that do, and keeps recovery automatic. An "Automatic key-frame request" switch in the APFPV settings (default on) alongside it would give people who care a way out. Option 2 alone would silently degrade recovery for most users, and option 3 is not enough given the credential is the camera's root login.

Two non-blocking observations for later: withDevice holds dev_mutex across a USB register write in WFB-NG mode, so releaseDevice on a detached adapter can stall for a libusb timeout (bounded, not a deadlock); and retune failure is detected only via exceptions, so a devourer retune that fails silently would still be recorded as success.

Approving. Please resolve the one remaining Qodo thread (the ruleset requires it) and this can merge.