AP_Filesystem: add @MAV_LOG virtual path for FTP log download#32860
AP_Filesystem: add @MAV_LOG virtual path for FTP log download#32860julianoes wants to merge 1 commit intoArduPilot:masterfrom
Conversation
Exposes the log directory as @MAV_LOG as specified in: github.com/mavlink/mavlink-devguide/pull/668.
|
Many boards with dataflash do not have logs on a filesystem and need the legacy protocol. I think we are also faster at the 'legacy' protocol. Do you plan to remove it from QGC? How does the user know how to choose? |
| // rewrite "foo/bar" to "<log_dir>/foo/bar" in the caller-supplied buffer. | ||
| // An empty fname resolves to the log directory itself. | ||
| // Returns false if the rewrite doesn't fit. | ||
| static bool rewrite_path(const char *fname, char *buf, size_t buflen) | ||
| { | ||
| const char *dir = log_directory(); | ||
| int n; | ||
| if (fname[0] == '\0') { | ||
| n = snprintf(buf, buflen, "%s", dir); | ||
| } else { | ||
| n = snprintf(buf, buflen, "%s/%s", dir, fname); | ||
| } | ||
| if (n < 0 || size_t(n) >= buflen) { | ||
| errno = ENAMETOOLONG; | ||
| return false; | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Have you considered putting this in the MAVFTP handler?
There is only one place to do it then rather than having to re-implement a whole filesystem.
But I guess that would require some funny gymnastics with listing.
There was a problem hiding this comment.
That sounds easier actually. Should I give it a try?
There was a problem hiding this comment.
Does the spec have any words on whether you are required to return such virtual items in the directory listing? AP already does several others, but none of the official spec items. If there is no requirement to return particular virtual directories when listing the root, this could be a very simple PR indeed.
But if this will hose legacy boards then maybe it's not worth it at all, or we will need the virtual filesystem to map block loggers to FTP. Like I said earlier I think it is slower. Maybe postpone on precise implementation until we work that out?
There was a problem hiding this comment.
I'm not sure the spec says that, it's all new, so we can still influence how this works.
My take would be that we don't have to list @MAV_LOG in root, because we already have /APM/LOGS (or /logs for PX4), so @MAV_LOG is only for GCSs that already know that @MAV_LOG could/should exist.
What do you think @hamishwillee?
There was a problem hiding this comment.
As you say @julianoes this is still open to discussion. The important thing for me here is that we have a compatible and documented interface.
I agree that we don't have to list @MAV_LOG - it's a directory alias so the actual directory is already listed. What do we think the benefit might be in doing so? I was thinking perhaps "to test if the feature is supported", but you might as well just try read the alias.
As an aside, should we make it explicit in the docs that you can't create a directory @Whatever using MAVFTP. My thinking is that we want to reserve potential aliases. Perhaps overthinking it.
3 participants
Summary
Exposes the log directory as @MAV_LOG as specified in: github.com/mavlink/mavlink-devguide/pull/668.
Implemented by Claude Code, tested manually by hand against mavlink/qgroundcontrol#14290.
Classification & Testing (check all that apply and add your own)
Description
This adds a virtual drive mapping from
@MAV_LOGto/APM/LOGS.FYI @peterbarker and @hamishwillee: one important caveat:
When using FTP instead of the LOG_* messages. We don't have functionality to transmit the file date/time (yet) using MAVLink FTP, so the logs are shown without date when listed via FTP. One way is to encode the date/time as part of the filename (which is what PX4 does). Alternatively, we could potentially extend MAVLink FTP to support list directory with date/time though. The latter solution would be the nicer, and specced way of doing things.