Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QGCCameraControl: allow to read camera information xml via ftp #10094

Merged
merged 3 commits into from
Aug 17, 2022

Conversation

dayjaby
Copy link
Contributor

@dayjaby dayjaby commented Dec 25, 2021

This PR heavily reuses the MAVLink FTP capabilities already present in QGroundControl and allows to specify camera definition URLs in CAMERA_INFORMATION with the mftp prefix, like mftp://[;comp=100]infos/camera_info.xml.

Tested it against the MAVSDK camera manager example PR: mavlink/MAVSDK#1655

TODO:

  • re-use component ID from CAMERA_INFORMATION as default for the ftp communication, as this would allow URLs like mftp://infos/camera_info.xml
  • test / handle communication aborts properly

@@ -2020,6 +2036,26 @@ QGCCameraControl::_downloadFinished()
//reply->deleteLater();
}

void QGCCameraControl::_ftpDownloadComplete(const QString& fileName, const QString& errorMsg)
{
qCDebug(CameraControlLog) << "QGCCameraControl::_ftpDownloadComplete fileName:errorMsg" << fileName << errorMsg;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to set class and function name, that can be configured via environment variable:
https://doc.qt.io/qt-5/debug.html

@@ -72,7 +72,11 @@ bool FTPManager::download(const QString& fromURI, const QString& toDir)
}
lastDirSlashIndex++; // move past slash

_downloadState.fileName = _downloadState.fullPathOnVehicle.right(_downloadState.fullPathOnVehicle.size() - lastDirSlashIndex);
if (fileName.size() == 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileName.isEmpty()

@@ -641,6 +645,8 @@ bool FTPManager::_parseURI(const QString& uri, QString& parsedURI, uint8_t& comp
if (!ok) {
qCWarning(FTPManagerLog) << "Incorrect format for component id" << uri;
return false;
} else {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont need else after return.

@@ -641,6 +645,8 @@ bool FTPManager::_parseURI(const QString& uri, QString& parsedURI, uint8_t& comp
if (!ok) {
qCWarning(FTPManagerLog) << "Incorrect format for component id" << uri;
return false;
} else {
qCDebug(FTPManagerLog) << "Found compId:" << (int)compId;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to cast it to int.

@dayjaby
Copy link
Contributor Author

dayjaby commented Jan 14, 2022

@patrickelectric Thanks for your review. I applied your suggested changes.

Copy link
Member

@patrickelectric patrickelectric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code appears to be correct

@mrpollo
Copy link
Member

mrpollo commented Feb 8, 2022

thanks for the contribution @dayjaby

@mrpollo mrpollo merged commit 7bcff05 into mavlink:master Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants