Skip to content

Download logs over MAVLink FTP, add ArduPilot support - #28

Closed
AlexKlimaj wants to merge 4 commits into
mainfrom
claude/logdata-mavlink-targeting-uj8d0y
Closed

Download logs over MAVLink FTP, add ArduPilot support#28
AlexKlimaj wants to merge 4 commits into
mainfrom
claude/logdata-mavlink-targeting-uj8d0y

Conversation

@AlexKlimaj

Copy link
Copy Markdown
Member

LOG_DATA (msg 120) carries no target_system/target_component, so a router
between logloader and the autopilot has no addressing information and copies
every chunk to every endpoint it serves -- the telemetry radio included. On a
node running mavlink-router a log download saturates links that have no
interest in it.

FILE_TRANSFER_PROTOCOL (msg 110) is addressed, and both PX4 and ArduPilot send
the reply back to the requesting sysid/compid, so the bulk transfer is unicast
to logloader. Switch the download path to MAVLink FTP. Nothing changes in
mavlink-router or in the autopilot.

The log list still comes from LOG_ENTRY: it is a handful of bytes per log
rather than megabytes, and it is the only source of the timestamp the databases
are keyed on, so existing uuids and sqlite state carry over untouched.

FtpLogFetcher probes /fs/microsd/log then /APM/LOGS to find the log directory
and the flight stack, indexes it, and maps each LOG_ENTRY onto a remote path.
An exact size match settles it when MAVSDK reports sizes; otherwise PX4 entries
resolve through the mtime LOG_ENTRY reports and ArduPilot entries through the
list ordering. Every download is size-checked before it is moved out of staging
and anything that does not line up falls back to LOG_DATA.

ArduPilot needed more than a new transport. MAVSDK's LogFiles plugin assumes
PX4's zero-based log ids -- it discards any LOG_ENTRY whose id is not below
num_logs -- while ArduPilot numbers list entries from one, so get_entries()
always returns NoLogfiles. LogEntryLister runs the same exchange over
MavlinkPassthrough without that assumption. Resolving a list entry to a file
then means reproducing ArduPilot's oldest-first ordering, which wraps at
LOG_MAX_FILES, so LASTLOG.TXT is read over FTP to find the wrap point.
Downloaded files pick up the .BIN extension.

download_protocol in config.toml selects "auto" (FTP with LOG_DATA fallback,
the default), "ftp" or "mavlink".

Also stop the download loop from spinning on a log it cannot fetch: it retried
immediately and only the slowness of a LOG_DATA timeout kept it from becoming
a hot loop.

Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01AsZc1HUPVUwpMCU8QxNR8p

LOG_DATA (msg 120) carries no target_system/target_component, so a router
between logloader and the autopilot has no addressing information and copies
every chunk to every endpoint it serves -- the telemetry radio included. On a
node running mavlink-router a log download saturates links that have no
interest in it.

FILE_TRANSFER_PROTOCOL (msg 110) is addressed, and both PX4 and ArduPilot send
the reply back to the requesting sysid/compid, so the bulk transfer is unicast
to logloader. Switch the download path to MAVLink FTP. Nothing changes in
mavlink-router or in the autopilot.

The log list still comes from LOG_ENTRY: it is a handful of bytes per log
rather than megabytes, and it is the only source of the timestamp the databases
are keyed on, so existing uuids and sqlite state carry over untouched.

FtpLogFetcher probes /fs/microsd/log then /APM/LOGS to find the log directory
and the flight stack, indexes it, and maps each LOG_ENTRY onto a remote path.
An exact size match settles it when MAVSDK reports sizes; otherwise PX4 entries
resolve through the mtime LOG_ENTRY reports and ArduPilot entries through the
list ordering. Every download is size-checked before it is moved out of staging
and anything that does not line up falls back to LOG_DATA.

ArduPilot needed more than a new transport. MAVSDK's LogFiles plugin assumes
PX4's zero-based log ids -- it discards any LOG_ENTRY whose id is not below
num_logs -- while ArduPilot numbers list entries from one, so get_entries()
always returns NoLogfiles. LogEntryLister runs the same exchange over
MavlinkPassthrough without that assumption. Resolving a list entry to a file
then means reproducing ArduPilot's oldest-first ordering, which wraps at
LOG_MAX_FILES, so LASTLOG.TXT is read over FTP to find the wrap point.
Downloaded files pick up the .BIN extension.

download_protocol in config.toml selects "auto" (FTP with LOG_DATA fallback,
the default), "ftp" or "mavlink".

Also stop the download loop from spinning on a log it cannot fetch: it retried
immediately and only the slowness of a LOG_DATA timeout kept it from becoming
a hot loop.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AsZc1HUPVUwpMCU8QxNR8p
@AlexKlimaj
AlexKlimaj requested a review from dakejahl July 26, 2026 03:58
dakejahl added 3 commits July 27, 2026 15:39
When the local flight-review server is down, the upload loop previously
probed and logged a failure for every pending log. Probe once, log a
single unreachable message with a 60s cooldown, and bail the current
upload batch (HTTP 503) instead of walking the queue.
Summarize live companion tests: FTP index/download, auto LOG_DATA
fallback on path size mismatch, large-file timeouts, and upload
cooldown when flight-review is offline.
Add remote_api_key config for ARK Flight Review (Authorization: Bearer and
X-API-Key per flight_review api_key.py). Empty key still attempts remote
upload without auth headers for open servers.

Treat any HTTP response as reachable so 302 home pages (review.arkelectron.com)
are not marked dead. Improve 401/403 handling with response body and stop
retry spam. Slightly longer connection timeouts for probe and upload.
@AlexKlimaj

Copy link
Copy Markdown
Member Author

@dakejahl added an API key option as well. This is ready for review.

@dakejahl

Copy link
Copy Markdown
Collaborator

Closing in favor of three stacked PRs that carry this work forward: #29, #30, #31.

Where each commit went

This PR Replacement
7a23c8e upload spam fix #30, cherry-picked unchanged
adc72da Flight Review API keys #31, cherry-picked with one change
397d3b3 MAVLink FTP transport superseded by #29, rewritten
a4e90d6 hardware test doc dropped (PR-scoped notes, kept in this thread)

The two upload commits are good as-is and merge on their own — they were only blocked behind the transport change. The one change in #31 is that 401/403 no longer blacklist a log permanently: those mean the account is not authorized yet, so the expected first-run state (upload_enabled = true before remote_api_key is set) would have discarded one pending log per cycle, permanently, and they would never upload once the key was added. The batch-stop is kept — same protection, recoverable.

Why the transport commit was rewritten rather than amended

The goal was to drop the LOG protocol entirely, and this PR still uses it for the log list on both stacks, with download_protocol = "auto" falling back to broadcast LOG_DATA when FTP path resolution misses. Keeping LOG_ENTRY as the source of truth is what forces the entry-to-file mapping layer — match by size, else PX4 mtime, else ArduPilot list ordering — and that layer is where the problems are:

  • match_by_size never fires on the pinned MAVSDK 3.17.1: mavlink_ftp_client.cpp truncates every listing entry at the tab, so ListDirectoryData has no sizes. Every resolution is a heuristic, and a miss transfers a whole wrong file before discarding it. The hardware test above shows exactly that on LOG0020.
  • LogEntryLister works around a MAVSDK bug fixed upstream in d2efa850 and shipped in v3.15.0 — the pinned 3.17.1 already handles ArduPilot's 1-based ids via _min_entry_id.
  • With the lister in play, the LOG_DATA fallback cannot work on ArduPilot at all: download_log_file_async validates entry.id against the plugin's own entry list, which never gets populated. That path is untested (noted in the test doc).

#29 removes the mapping layer instead of fixing it: the FTP directory listing becomes the source of truth, identity is remote path + size, and ArduPilot needs no special handling because a listing just reports files. LogEntryLister, LASTLOG.TXT ordering, download_protocol and log_extension are all gone. It is the shape QGC's Onboard Logs (FTP) tab already uses.

Since MAVSDK drops size/mtime from listings, #29 runs the listing over MavlinkPassthrough and keeps the full entry. That is temporary — mavlink/MAVSDK#2891 adds Ftp::FilesystemEntry with exactly those fields, and it is on MAVSDK main but not in any release through v3.17.2, so it can be deleted once a release carries it.

Verified against PX4 SITL: @MAV_LOG probe, indexing, burst download with byte-identical files, ListDirectoryWithTime and the path-derived fallback, the directory override, and a legacy DB migration that keeps already-uploaded logs from being re-fetched.

Two things from this PR that #29 also picked up, both of which the hardware test surfaced: the actively-written log is now held back until two listings agree on its size (rather than downloaded, size-checked, discarded and retried forever), and FTP sessions are reset at connect, which is what left the FC refusing transfers until reboot after a mid-session kill.

The large-file FTP timeout noted here is still open — #29 keeps one bad log from blocking the queue, but transfer resume is worth a follow-up.

@dakejahl dakejahl closed this Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants