Skip to content

Commit

Permalink
mavlinkftp:// -> mftp://, add support for comp id in uri (#9225)
Browse files Browse the repository at this point in the history
  • Loading branch information
@DonLakeFlyer
DonLakeFlyer authored Dec 1, 2020
1 parent 1389ec6 commit 4b50ffd
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 28 deletions.
49 changes: 39 additions & 10 deletions src/Vehicle/ComponentInformationManager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "CompInfoVersion.h"
#include "CompInfoParam.h"
#include "QGCFileDownload.h"
#include "QGCApplication.h"

#include <QStandardPaths>
#include <QJsonDocument>
Expand Down Expand Up @@ -181,12 +182,12 @@ void RequestMetaDataTypeStateMachine::_stateRequestCompInfo(StateMachine* stateM
WeakLinkInterfacePtr weakLink = vehicle->vehicleLinkManager()->primaryLink();

if (weakLink.expired()) {
qCDebug(ComponentInformationManagerLog) << QStringLiteral("_stateRequestCompInfo Skipping component information % 1 request due to no primary link").arg(requestMachine->typeToString());
qCDebug(ComponentInformationManagerLog) << QStringLiteral("_stateRequestCompInfo Skipping component information %1 request due to no primary link").arg(requestMachine->typeToString());
stateMachine->advance();
} else {
SharedLinkInterfacePtr sharedLink = weakLink.lock();
if (sharedLink->linkConfiguration()->isHighLatency() || sharedLink->isPX4Flow() || sharedLink->isLogReplay()) {
qCDebug(ComponentInformationManagerLog) << QStringLiteral("_stateRequestCompInfo Skipping component information % 1 request due to link type").arg(requestMachine->typeToString());
qCDebug(ComponentInformationManagerLog) << QStringLiteral("_stateRequestCompInfo Skipping component information %1 request due to link type").arg(requestMachine->typeToString());
stateMachine->advance();
} else {
qCDebug(ComponentInformationManagerLog) << "Requesting component information" << requestMachine->typeToString();
Expand Down Expand Up @@ -226,6 +227,9 @@ void RequestMetaDataTypeStateMachine::_ftpDownloadCompleteMetaDataJson(const QSt
disconnect(_compInfo->vehicle->ftpManager(), &FTPManager::downloadComplete, this, &RequestMetaDataTypeStateMachine::_ftpDownloadCompleteMetaDataJson);
if (errorMsg.isEmpty()) {
_jsonMetadataFileName = _downloadCompleteJsonWorker(fileName, "metadata.json");
} else if (qgcApp()->runningUnitTests()) {
// Unit test should always succeed
qCWarning(ComponentInformationManagerLog) << "RequestMetaDataTypeStateMachine::_ftpDownloadCompleteMetaDataJson failed filename:errorMsg" << fileName << errorMsg;
}

advance();
Expand All @@ -240,6 +244,9 @@ void RequestMetaDataTypeStateMachine::_ftpDownloadCompleteTranslationJson(const
disconnect(_compInfo->vehicle->ftpManager(), &FTPManager::downloadComplete, this, &RequestMetaDataTypeStateMachine::_ftpDownloadCompleteTranslationJson);
if (errorMsg.isEmpty()) {
_jsonTranslationFileName = _downloadCompleteJsonWorker(fileName, "translation.json");
} else if (qgcApp()->runningUnitTests()) {
// Unit test should always succeed
qCWarning(ComponentInformationManagerLog) << "RequestMetaDataTypeStateMachine::_ftpDownloadCompleteTranslationJson failed filename:errorMsg" << fileName << errorMsg;
}

advance();
Expand All @@ -252,6 +259,9 @@ void RequestMetaDataTypeStateMachine::_httpDownloadCompleteMetaDataJson(QString
disconnect(qobject_cast<QGCFileDownload*>(sender()), &QGCFileDownload::downloadComplete, this, &RequestMetaDataTypeStateMachine::_httpDownloadCompleteMetaDataJson);
if (errorMsg.isEmpty()) {
_jsonMetadataFileName = _downloadCompleteJsonWorker(localFile, "metadata.json");
} else if (qgcApp()->runningUnitTests()) {
// Unit test should always succeed
qCWarning(ComponentInformationManagerLog) << "RequestMetaDataTypeStateMachine::_httpDownloadCompleteMetaDataJson failed remoteFile:localFile:errorMsg" << remoteFile << localFile << errorMsg;
}

advance();
Expand All @@ -266,6 +276,9 @@ void RequestMetaDataTypeStateMachine::_httpDownloadCompleteTranslationJson(QStri
disconnect(qobject_cast<QGCFileDownload*>(sender()), &QGCFileDownload::downloadComplete, this, &RequestMetaDataTypeStateMachine::_httpDownloadCompleteTranslationJson);
if (errorMsg.isEmpty()) {
_jsonTranslationFileName = _downloadCompleteJsonWorker(localFile, "translation.json");
} else if (qgcApp()->runningUnitTests()) {
// Unit test should always succeed
qCWarning(ComponentInformationManagerLog) << "RequestMetaDataTypeStateMachine::_httpDownloadCompleteTranslationJson failed remoteFile:localFile:errorMsg" << remoteFile << localFile << errorMsg;
}

advance();
Expand All @@ -279,13 +292,21 @@ void RequestMetaDataTypeStateMachine::_stateRequestMetaDataJson(StateMachine* st

if (compInfo->available) {
qCDebug(ComponentInformationManagerLog) << "Downloading metadata json" << compInfo->uriMetaData;
if (_uriIsFTP(compInfo->uriMetaData)) {
if (_uriIsMAVLinkFTP(compInfo->uriMetaData)) {
connect(ftpManager, &FTPManager::downloadComplete, requestMachine, &RequestMetaDataTypeStateMachine::_ftpDownloadCompleteMetaDataJson);
ftpManager->download(compInfo->uriMetaData, QStandardPaths::writableLocation(QStandardPaths::TempLocation));
if (!ftpManager->download(compInfo->uriMetaData, QStandardPaths::writableLocation(QStandardPaths::TempLocation))) {
qCWarning(ComponentInformationManagerLog) << "RequestMetaDataTypeStateMachine::_stateRequestMetaDataJson FTPManager::download returned failure";
disconnect(ftpManager, &FTPManager::downloadComplete, requestMachine, &RequestMetaDataTypeStateMachine::_ftpDownloadCompleteMetaDataJson);
requestMachine->advance();
}
} else {
QGCFileDownload* download = new QGCFileDownload(requestMachine);
connect(download, &QGCFileDownload::downloadComplete, requestMachine, &RequestMetaDataTypeStateMachine::_httpDownloadCompleteMetaDataJson);
download->download(compInfo->uriMetaData);
if (!download->download(compInfo->uriMetaData)) {
qCWarning(ComponentInformationManagerLog) << "RequestMetaDataTypeStateMachine::_stateRequestMetaDataJson QGCFileDownload::download returned failure";
disconnect(download, &QGCFileDownload::downloadComplete, requestMachine, &RequestMetaDataTypeStateMachine::_httpDownloadCompleteMetaDataJson);
requestMachine->advance();
}
}
} else {
qCDebug(ComponentInformationManagerLog) << "Skipping metadata json download. Component information not available";
Expand All @@ -305,13 +326,21 @@ void RequestMetaDataTypeStateMachine::_stateRequestTranslationJson(StateMachine*
requestMachine->advance();
} else {
qCDebug(ComponentInformationManagerLog) << "Downloading translation json" << compInfo->uriTranslation;
if (_uriIsFTP(compInfo->uriTranslation)) {
if (_uriIsMAVLinkFTP(compInfo->uriTranslation)) {
connect(ftpManager, &FTPManager::downloadComplete, requestMachine, &RequestMetaDataTypeStateMachine::_ftpDownloadCompleteTranslationJson);
ftpManager->download(compInfo->uriTranslation, QStandardPaths::writableLocation(QStandardPaths::TempLocation));
if (!ftpManager->download(compInfo->uriTranslation, QStandardPaths::writableLocation(QStandardPaths::TempLocation))) {
qCWarning(ComponentInformationManagerLog) << "_stateRequestTranslationJson::_stateRequestMetaDataJson FTPManager::download returned failure";
connect(ftpManager, &FTPManager::downloadComplete, requestMachine, &RequestMetaDataTypeStateMachine::_ftpDownloadCompleteTranslationJson);
requestMachine->advance();
}
} else {
QGCFileDownload* download = new QGCFileDownload(requestMachine);
connect(download, &QGCFileDownload::downloadComplete, requestMachine, &RequestMetaDataTypeStateMachine::_httpDownloadCompleteTranslationJson);
download->download(compInfo->uriTranslation);
if (!download->download(compInfo->uriTranslation)) {
qCWarning(ComponentInformationManagerLog) << "_stateRequestTranslationJson::_stateRequestMetaDataJson QGCFileDownload::download returned failure";
disconnect(download, &QGCFileDownload::downloadComplete, requestMachine, &RequestMetaDataTypeStateMachine::_httpDownloadCompleteTranslationJson);
requestMachine->advance();
}
}
}
} else {
Expand All @@ -337,7 +366,7 @@ void RequestMetaDataTypeStateMachine::_stateRequestComplete(StateMachine* stateM
requestMachine->advance();
}

bool RequestMetaDataTypeStateMachine::_uriIsFTP(const QString& uri)
bool RequestMetaDataTypeStateMachine::_uriIsMAVLinkFTP(const QString& uri)
{
return uri.startsWith("mavlinkftp", Qt::CaseInsensitive);
return uri.startsWith(QStringLiteral("%1://").arg(FTPManager::mavlinkFTPScheme), Qt::CaseInsensitive);
}
2 changes: 1 addition & 1 deletion src/Vehicle/ComponentInformationManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ private slots:
static void _stateRequestMetaDataJson (StateMachine* stateMachine);
static void _stateRequestTranslationJson (StateMachine* stateMachine);
static void _stateRequestComplete (StateMachine* stateMachine);
static bool _uriIsFTP (const QString& uri);
static bool _uriIsMAVLinkFTP (const QString& uri);


ComponentInformationManager* _compMgr = nullptr;
Expand Down
51 changes: 41 additions & 10 deletions src/Vehicle/FTPManager.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

QGC_LOGGING_CATEGORY(FTPManagerLog, "FTPManagerLog")

const char* FTPManager::mavlinkFTPScheme = "mftp";

FTPManager::FTPManager(Vehicle* vehicle)
: QObject (vehicle)
, _vehicle (vehicle)
Expand All @@ -32,9 +34,9 @@ FTPManager::FTPManager(Vehicle* vehicle)
Q_ASSERT(sizeof(MavlinkFTP::RequestHeader) == 12);
}

bool FTPManager::download(const QString& from, const QString& toDir)
bool FTPManager::download(const QString& fromURI, const QString& toDir)
{
qCDebug(FTPManagerLog) << "download from:" << from << "to:" << toDir;
qCDebug(FTPManagerLog) << "download fromURI:" << fromURI << "to:" << toDir;

if (!_rgStateMachine.isEmpty()) {
qCDebug(FTPManagerLog) << "Cannot download. Already in another operation";
Expand All @@ -55,11 +57,9 @@ bool FTPManager::download(const QString& from, const QString& toDir)
_downloadState.reset();
_downloadState.toDir.setPath(toDir);

QString ftpPrefix("mavlinkftp://");
if (from.startsWith(ftpPrefix, Qt::CaseInsensitive)) {
_downloadState.fullPathOnVehicle = from.right(from.length() - ftpPrefix.length() + 1);
} else {
_downloadState.fullPathOnVehicle = from;
if (!_parseURI(fromURI, _downloadState.fullPathOnVehicle, _ftpCompId)) {
qCWarning(FTPManagerLog) << "_parseURI failed";
return false;
}

// We need to strip off the file name from the fully qualified path. We can't use the usual QDir
Expand All @@ -74,7 +74,7 @@ bool FTPManager::download(const QString& from, const QString& toDir)

_downloadState.fileName = _downloadState.fullPathOnVehicle.right(_downloadState.fullPathOnVehicle.size() - lastDirSlashIndex);

qDebug() << _downloadState.fullPathOnVehicle << _downloadState.fileName;
qCDebug(FTPManagerLog) << "_downloadState.fullPathOnVehicle:_downloadState.fileName" << _downloadState.fullPathOnVehicle << _downloadState.fileName;

_startStateMachine();

Expand Down Expand Up @@ -105,7 +105,7 @@ void FTPManager::_downloadComplete(const QString& errorMsg)

void FTPManager::_mavlinkMessageReceived(const mavlink_message_t& message)
{
if (message.msgid != MAVLINK_MSG_ID_FILE_TRANSFER_PROTOCOL) {
if (message.msgid != MAVLINK_MSG_ID_FILE_TRANSFER_PROTOCOL && message.compid != _ftpCompId) {
return;
}

Expand Down Expand Up @@ -548,8 +548,39 @@ void FTPManager::_sendRequestExpectAck(MavlinkFTP::Request* request)
&message,
0, // Target network, 0=broadcast?
_vehicle->id(),
MAV_COMP_ID_AUTOPILOT1,
_ftpCompId,
(uint8_t*)request); // Payload
_vehicle->sendMessageOnLinkThreadSafe(sharedLink.get(), message);
}
}

bool FTPManager::_parseURI(const QString& uri, QString& parsedURI, uint8_t& compId)
{
parsedURI = uri;
compId = MAV_COMP_ID_AUTOPILOT1;

// Pull scheme off the front if there
QString ftpPrefix(QStringLiteral("%1://").arg(mavlinkFTPScheme));
if (parsedURI.startsWith(ftpPrefix, Qt::CaseInsensitive)) {
parsedURI = parsedURI.right(parsedURI.length() - ftpPrefix.length() + 1);
}
if (parsedURI.contains("://")) {
qCWarning(FTPManagerLog) << "Incorrect uri scheme or format" << uri;
return false;
}

// Pull component id off the front if there
QRegularExpression regEx("^/??\\[\\;comp\\=(\\d+)\\]");
QRegularExpressionMatch match = regEx.match(parsedURI);
if (match.hasMatch()) {
bool ok;
compId = match.captured(1).toUInt(&ok);
if (!ok) {
qCWarning(FTPManagerLog) << "Incorrect format for component id" << uri;
return false;
}
parsedURI.replace(QRegularExpression("\\[\\;comp\\=\\d+\\]"), "");
}

return true;
}
9 changes: 7 additions & 2 deletions src/Vehicle/FTPManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,14 @@ class FTPManager : public QObject
FTPManager(Vehicle* vehicle);

/// Downloads the specified file.
/// @param from File to download from vehicle, fully qualified path. May be in the format mavlinkftp://...
/// @param fromURI File to download from vehicle, fully qualified path. May be in the format "mftp://[;comp=<id>]..." where the component id is specified.
/// If component id is not specified MAV_COMP_ID_AUTOPILOT1 is used.
/// @param toDir Local directory to download file to
/// @return true: download has started, false: error, no download
/// Signals downloadComplete, commandError, commandProgress
bool download(const QString& from, const QString& toDir);
bool download(const QString& fromURI, const QString& toDir);

static const char* mavlinkFTPScheme;

signals:
void downloadComplete(const QString& file, const QString& errorMsg);
Expand Down Expand Up @@ -120,8 +123,10 @@ private slots:
void _fillRequestDataWithString(MavlinkFTP::Request* request, const QString& str);
void _fillMissingBlocksWorker (bool firstRequest);
void _burstReadFileWorker (bool firstRequest);
bool _parseURI (const QString& uri, QString& parsedURI, uint8_t& compId);

Vehicle* _vehicle;
uint8_t _ftpCompId;
QList<StateFunctions_t> _rgStateMachine;
DownloadState_t _downloadState;
QTimer _ackOrNakTimeoutTimer;
Expand Down
14 changes: 11 additions & 3 deletions src/Vehicle/VehicleLinkManagerTest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ void VehicleLinkManagerTest::cleanup(void)
QCOMPARE(_linkManager->links().count(), 0);
}

_multiVehicleMgr = nullptr;
_multiVehicleMgr = nullptr;

UnitTest::cleanup();
}
Expand All @@ -73,12 +73,14 @@ void VehicleLinkManagerTest::_simpleLinkTest(void)
Vehicle* vehicle = _multiVehicleMgr->activeVehicle();
QVERIFY(vehicle);
QSignalSpy spyVehicleDelete(vehicle, &QObject::destroyed);
QSignalSpy spyVehicleInitialConnectComplete(vehicle, &Vehicle::initialConnectComplete);

QCOMPARE(mockConfig.use_count(), 2); // Refs: This method, MockLink
QCOMPARE(mockLink.use_count(), 3); // Refs: This method, LinkManager, Vehicle

// We explicitly don't wait for the Vehicle to finish it's startup sequence before we disconnect.
// This helps to wring out possible crashing when the link goes down before the startup sequence is complete.
// We wait for the full initial connect sequence to complete to catch anby ComponentInformationManager bugs
QCOMPARE(spyVehicleInitialConnectComplete.wait(3000), true);

mockLink->disconnect();

// Vehicle should go away due to disconnect
Expand Down Expand Up @@ -113,6 +115,8 @@ void VehicleLinkManagerTest::_simpleCommLossTest(void)
QCOMPARE(_multiVehicleMgr->vehicles()->count(), 1);
Vehicle* vehicle = _multiVehicleMgr->activeVehicle();
QVERIFY(vehicle);
QSignalSpy spyVehicleInitialConnectComplete(vehicle, &Vehicle::initialConnectComplete);
QCOMPARE(spyVehicleInitialConnectComplete.wait(3000), true);

QSignalSpy spyCommLostChanged(vehicle->vehicleLinkManager(), &VehicleLinkManager::communicationLostChanged);
pMockLink->setCommLost(true);
Expand Down Expand Up @@ -157,6 +161,8 @@ void VehicleLinkManagerTest::_multiLinkSingleVehicleTest(void)
VehicleLinkManager* vehicleLinkManager = vehicle->vehicleLinkManager();
QVERIFY(vehicle);
QVERIFY(vehicleLinkManager);
QSignalSpy spyVehicleInitialConnectComplete(vehicle, &Vehicle::initialConnectComplete);
QCOMPARE(spyVehicleInitialConnectComplete.wait(3000), true);

QCOMPARE(mockLink1.get(), vehicleLinkManager->primaryLink().lock().get());

Expand Down Expand Up @@ -239,6 +245,8 @@ void VehicleLinkManagerTest::_connectionRemovedTest(void)
QCOMPARE(spyVehicleCreate.wait(1000), true);
Vehicle* vehicle = _multiVehicleMgr->activeVehicle();
QVERIFY(vehicle);
QSignalSpy spyVehicleInitialConnectComplete(vehicle, &Vehicle::initialConnectComplete);
QCOMPARE(spyVehicleInitialConnectComplete.wait(3000), true);

QSignalSpy spyCommLostChanged(vehicle->vehicleLinkManager(), &VehicleLinkManager::communicationLostChanged);

Expand Down
4 changes: 2 additions & 2 deletions src/comm/MockLink.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1619,7 +1619,7 @@ void MockLink::_sendVersionMetaData(void)
{
mavlink_message_t responseMsg;
#if 1
char metaDataURI[MAVLINK_MSG_COMPONENT_INFORMATION_FIELD_METADATA_URI_LEN] = "mavlinkftp://version.json.gz";
char metaDataURI[MAVLINK_MSG_COMPONENT_INFORMATION_FIELD_METADATA_URI_LEN] = "mftp://[;comp=1]version.json.gz";
#else
char metaDataURI[MAVLINK_MSG_COMPONENT_INFORMATION_FIELD_METADATA_URI_LEN] = "https://bit.ly/31nm0fs";
#endif
Expand All @@ -1642,7 +1642,7 @@ void MockLink::_sendParameterMetaData(void)
{
mavlink_message_t responseMsg;
#if 1
char metaDataURI[MAVLINK_MSG_COMPONENT_INFORMATION_FIELD_METADATA_URI_LEN] = "mavlinkftp://parameter.json";
char metaDataURI[MAVLINK_MSG_COMPONENT_INFORMATION_FIELD_METADATA_URI_LEN] = "mftp://[;comp=1]parameter.json";
#else
char metaDataURI[MAVLINK_MSG_COMPONENT_INFORMATION_FIELD_METADATA_URI_LEN] = "https://bit.ly/2ZKRIRE";
#endif
Expand Down

0 comments on commit 4b50ffd

Please sign in to comment.