From 07df322e35e79086d84b4b0579b68a50b2f4b8f2 Mon Sep 17 00:00:00 2001 From: Julian Oes Date: Thu, 3 Sep 2026 14:51:30 +1200 Subject: [PATCH] fix(FTPManager): fall back to non-burst read when a burst cannot complete A burst read streams packets until the end of the file and the protocol gives the client no way to ask for less, so a link which cannot absorb the stream delivers only the first few packets of every burst. Retrying the burst then makes the same small amount of progress each time and the download fails, even though every attempt demonstrably moved forward. Seen downloading a camera definition from a MAVSDK camera routed through PX4: PX4 forwards one message per loop iteration into a two-message buffer, so exactly two of the fourteen burst packets survived each round, and QGC gave up after three retries with a third of the file fetched. Record whatever has not arrived as missing and advance to the existing fill-missing-blocks state instead of failing. That path already requests one chunk at a time with kCmdReadFile and never has more than a single message in flight, so it is unaffected by whatever truncated the burst. It also covers a burst that delivers nothing at all, which makes the whole file one missing block. Healthy links are unaffected since this is only reached once the burst has already exhausted its retries. --- src/Vehicle/FTPManager.cc | 27 +++++++++++++++++++++++++++ src/Vehicle/FTPManager.h | 2 ++ 2 files changed, 29 insertions(+) diff --git a/src/Vehicle/FTPManager.cc b/src/Vehicle/FTPManager.cc index 2087c60f799..afa5f08a902 100644 --- a/src/Vehicle/FTPManager.cc +++ b/src/Vehicle/FTPManager.cc @@ -910,9 +910,36 @@ void FTPManager::_burstReadFileAckOrNak(const MavlinkFTP::Request* ackOrNak) } } +void FTPManager::_recordMissingTail(void) +{ + if ((_downloadState.fileSize == 0) || (_downloadState.expectedOffset >= _downloadState.fileSize)) { + return; + } + + MissingData_t missingData; + missingData.offset = _downloadState.expectedOffset; + missingData.cBytesMissing = _downloadState.fileSize - _downloadState.expectedOffset; + _downloadState.rgMissingData.append(missingData); + + qCDebug(FTPManagerLog) << "_recordMissingTail: offset:cBytesMissing" << missingData.offset << missingData.cBytesMissing; +} + void FTPManager::_burstReadFileTimeout(void) { if (++_downloadState.retryCount > _maxRetry) { + // A burst streams packets until the file ends, with no way for us to ask for less. Links + // which can't absorb that (a router with a shallow forwarding queue, for example) deliver + // only the first few packets of every burst, so retrying the burst makes the same small + // amount of progress each time and then gives up. Fall back to the non-burst read used to + // repair holes instead: it fetches one chunk per request and never has more than a single + // message in flight. + _recordMissingTail(); + if (!_downloadState.rgMissingData.isEmpty()) { + qCDebug(FTPManagerLog) << "_burstReadFileTimeout: retries exceeded, falling back to non-burst read"; + _advanceStateMachine(); + return; + } + qCDebug(FTPManagerLog) << QString("_burstReadFileTimeout retries exceeded"); _downloadComplete(tr("Download failed")); } else { diff --git a/src/Vehicle/FTPManager.h b/src/Vehicle/FTPManager.h index 2e1a63b08af..68b560ab878 100644 --- a/src/Vehicle/FTPManager.h +++ b/src/Vehicle/FTPManager.h @@ -220,6 +220,8 @@ private slots: void _fillRequestDataWithString(MavlinkFTP::Request* request, const QString& str); void _fillMissingBlocksWorker (bool firstRequest); void _burstReadFileWorker (bool firstRequest); + /// Records anything not yet received as missing so the non-burst read can fetch it. + void _recordMissingTail (void); void _listDirectoryWorker (bool firstRequest); bool _parseURI (uint8_t fromCompId, const QString& uri, QString& parsedURI, uint8_t& compId); void _listDirectoryCompleteNoError(void) { _listDirectoryComplete(QString()); }