feat(logloader): download logs over MAVLink FTP only - #29
Closed
dakejahl wants to merge 1 commit into
Closed
Conversation
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. Move both halves of the job to MAVLink FTP: the listing that finds the logs and the transfer that downloads them. FILE_TRANSFER_PROTOCOL (msg 110) is addressed and both PX4 and ArduPilot reply to the requesting sysid/compid, so everything is unicast to logloader. The classic log protocol is gone entirely -- no LOG_REQUEST_LIST, no LOG_ENTRY, no LOG_DATA, and no MAVSDK LogFiles plugin. Dropping LOG_ENTRY means dropping the only thing that identified a log, so identity moves to the listing: the path below the log root plus the size. That is also what makes ArduPilot work without special cases. Its list entry numbering, its LOG_MAX_FILES wrap and its .BIN extension were all consequences of the old protocol; a directory listing just reports files. The log root is probed at "@MAV_LOG" -- the virtual directory the MAVLink FTP specification defines for this -- then at /fs/microsd/log and /APM/LOGS for firmware that predates it. Timestamps come from the listing when the vehicle implements ListDirectoryWithTime, otherwise from the start time PX4 encodes in the path; ArduPilot logs simply have no timestamp and nothing needs one. MAVSDK's Ftp plugin drops the size and modification time from list replies, so FtpListClient runs ListDirectory / ListDirectoryWithTime over MavlinkPassthrough and keeps the whole entry. Bulk transfers still go through the plugin. Databases written by earlier versions keyed logs on the LOG_ENTRY timestamp, which FTP cannot reproduce. Their rows are set aside as logs_legacy on first start and matched to the listing by size, so a fleet upgrading to this does not re-download and re-upload its history. Also: - A log is only queued once two consecutive listings agree on its size, so the log being written right now is not downloaded at a size it will not keep. - Downloads are staged and size-checked before being moved next to the finished logs, and a log that cannot be fetched sorts to the back of the queue instead of blocking the ones behind it. - Open FTP sessions are reset at connect, which is what left PX4 refusing transfers until reboot after logloader was killed mid-download. - The databases are created before they are opened, which failed when application_directory did not exist yet.
dakejahl
force-pushed
the
feat/mavlink-ftp-only
branch
from
July 28, 2026 03:22
9dc4104 to
b4d4d90
Compare
This was referenced Jul 28, 2026
Collaborator
Author
|
Superseded by #32, which contains this branch's commits unchanged and with authorship intact — merging #32 merges this. Reviewing this stack turned up structural problems in the code underneath it, and fixing those meant rewriting the parts these PRs sit on. Keeping them open would have meant reviewing #29's Nothing here is dropped: the FTP transport, the reachability cooldown and the API-key headers are all in #32. The one deliberate change is that 401/403 no longer blacklist a log — see the note in #32. |
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
Supersedes the transport half of #28. Replaces the classic MAVLink log protocol with MAVLink FTP for both the log listing and the bulk transfer, and adds ArduPilot support as a consequence rather than as a special case.
#28 should stay open for its other two commits — the upload-spam fix and the Flight Review API key — which are independent of the transport and good as-is (minus the 401/403 blacklisting, see the review notes on that PR).
Problem
LOG_DATA(msg 120) has notarget_system/target_component. A router between logloader and the autopilot has no addressing information to work with, so it copies every chunk to every endpoint it serves, telemetry radio included. On a node running mavlink-router a log download saturates links that have no interest in it.LOG_ENTRYalso forces a second problem: it is the only thing that identifies a log, and it reports an id and a timestamp rather than a file. Anything that downloads over FTP while listing over the log protocol has to map one onto the other by size, mtime or list ordering, and ArduPilot's list-entry numbering, itsLOG_MAX_FILESwrap and its.BINnaming all have to be modelled to do it. That mapping layer is where the complexity and the failure modes live: when it guesses wrong, the wrong file is transferred in full and then discarded.Solution
Use MAVLink FTP for both halves.
FILE_TRANSFER_PROTOCOL(msg 110) is addressed and both stacks reply to the requesting sysid/compid, so transfers are unicast. The log protocol is gone entirely — noLOG_REQUEST_LIST, noLOG_ENTRY, noLOG_DATA, no MAVSDKLogFilesplugin.Identity moves to the listing: path below the log root plus size. There is no cross-protocol mapping left to get wrong, and ArduPilot needs no special handling — a directory listing just reports files. This is the shape QGC's Onboard Logs (FTP) tab already uses.
The root is probed at
@MAV_LOG(the virtual directory from the FTP spec), then/fs/microsd/logand/APM/LOGSfor older firmware. Timestamps come from the listing when the vehicle implementsListDirectoryWithTime, otherwise from the start time PX4 encodes in the path; ArduPilot logs have none and nothing needs one.MAVSDK's
Ftpplugin discards size and mtime from list replies, soFtpListClientruns the listing overMavlinkPassthroughand keeps the full entry. Bulk transfers still use the plugin. Worth upstreaming to MAVSDK later so this can be deleted.Pre-FTP databases keyed logs on the
LOG_ENTRYtimestamp, which FTP cannot reproduce. Those rows are set aside aslogs_legacyon first start and matched to the listing by size, so a fleet upgrading to this does not re-download and re-upload its history.Along the way: a log is only queued once two consecutive listings agree on its size (so the log being written right now is never fetched at a size it will not keep); downloads are staged and size-checked before being moved into place; a log that cannot be fetched sorts to the back of the queue instead of blocking everything behind it; FTP sessions are reset at connect, which is what left PX4 refusing transfers until reboot after logloader was killed mid-download.
Verified against PX4 SITL (
main, MAVSDK 3.17.1)@MAV_LOGprobe, session reset, index, burst download; files byte-identical to the vehicle's, staging directory clean afterwards.ListDirectoryWithTime, and via the path-derived fallback when the opcode is forced off (the path ArduPilot takes).remote_log_directoryoverride against a physical path.3 of 4 indexed, 1 still being written) instead of being downloaded and discarded.Follow-ups
-x; FTP is now load-bearing.