From a499d17eece8eaa8ec744a2d490e0da455cbf95b Mon Sep 17 00:00:00 2001 From: Bob Long Date: Tue, 24 Mar 2026 12:10:05 +1100 Subject: [PATCH] GCS_MAVLink: burst reply offset calculation fix We were calculating the reply offset using the index number times the requested read size, which has an unwritten assumption that the filesystem will never return a short non-zero read for any reason other than the end of the file, which might not be true for all virtual file backends (including future ones we might want to add). Additionally, the burst_complete flag makes the same flawed assumption. The burst_complete flag is only needed when we hit the burst packet limit or the NAK at the end of the file. It is not needed on the final ACK prior to the EOF NAK, and in fact, can lead to a pointless extra burst read from the client. As a side-effect, this fixes the offset calculation for the NAK on in two edge cases: - File size is an exact multiple of the read size - The request is at-or-beyond the end of the file, in which case, we should send the requested offset back in the NAK --- libraries/GCS_MAVLink/GCS_FTP.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/libraries/GCS_MAVLink/GCS_FTP.cpp b/libraries/GCS_MAVLink/GCS_FTP.cpp index 3a2b6da99f4..09875323d75 100644 --- a/libraries/GCS_MAVLink/GCS_FTP.cpp +++ b/libraries/GCS_MAVLink/GCS_FTP.cpp @@ -592,10 +592,12 @@ bool GCS_FTP::Session::handle_request(Transaction &request, Transaction &reply) // this transfer size is enough for a full parameter file with max parameters const uint32_t transfer_size = 2000; + reply.offset = request.offset; for (uint32_t i = 0; (i < transfer_size); i++) { // fill the buffer const ssize_t read_bytes = AP::FS().read(fd, reply.data, MIN(sizeof(reply.data), max_read)); if (read_bytes == -1) { + reply.burst_complete = true; GCS_FTP::error(reply, FTP_ERROR::FailErrno); break; } @@ -606,21 +608,20 @@ bool GCS_FTP::Session::handle_request(Transaction &request, Transaction &reply) } if (read_bytes == 0) { + reply.burst_complete = true; GCS_FTP::error(reply, FTP_ERROR::EndOfFile); break; } reply.opcode = FTP_OP::Ack; - reply.offset = request.offset + i * max_read; - reply.burst_complete = ((read_bytes < max_read) || (i == (transfer_size - 1))); + // Signal to the client that they need to request another burst read to get more data + reply.burst_complete = (i == (transfer_size - 1)); reply.size = (uint8_t)read_bytes; push_reply(reply); - if (read_bytes < max_read) { - // ensure the NACK which we send next is at the right offset - reply.offset += read_bytes; - } + // update the offset for the next read + reply.offset += read_bytes; // prep the reply to be used again reply.seq_number++;