Merge pull request #6076 from DonLakeFlyer/BatchTerrainStable

Batch terrain requests

src/MissionManager/MissionSettingsItem.cc

@@ -57,7 +57,6 @@ MissionSettingsItem::MissionSettingsItem(Vehicle* vehicle, QObject* parent)
 
     connect(this,               &MissionSettingsItem::terrainAltitudeChanged,           this, &MissionSettingsItem::_setHomeAltFromTerrain);
 
-    connect(&_plannedHomePositionAltitudeFact,  &Fact::valueChanged,                    this, &MissionSettingsItem::_setDirty);
     connect(&_plannedHomePositionAltitudeFact,  &Fact::valueChanged,                    this, &MissionSettingsItem::_updateAltitudeInCoordinate);
 
     connect(&_cameraSection,    &CameraSection::dirtyChanged,   this, &MissionSettingsItem::_sectionDirtyChanged);
@@ -297,11 +296,6 @@ void MissionSettingsItem::setMissionEndRTL(bool missionEndRTL)
 void MissionSettingsItem::_setHomeAltFromTerrain(double terrainAltitude)
 {
     if (!_plannedHomePositionFromVehicle) {
-		// We need to stop this from signalling, Otherwise the dirty but get set on a delay
-		// which then marks the Plan view as incorrectly dirty
-        _plannedHomePositionAltitudeFact.setSendValueChangedSignals(false);
         _plannedHomePositionAltitudeFact.setRawValue(terrainAltitude);
-        _plannedHomePositionAltitudeFact.clearDeferredValueChangeSignal();
-        _plannedHomePositionAltitudeFact.setSendValueChangedSignals(true);
     }
 }

src/MissionManager/VisualMissionItem.h

@@ -7,9 +7,7 @@
  *
  ****************************************************************************/
 
-
-#ifndef VisualMissionItem_H
-#define VisualMissionItem_H
+#pragma once
 
 #include <QObject>
 #include <QString>
@@ -215,5 +213,3 @@ private:
     double _lastLatTerrainQuery;
     double _lastLonTerrainQuery;
 };
-
-#endif

src/Terrain.cc

@@ -17,27 +17,63 @@
 #include <QJsonDocument>
 #include <QJsonObject>
 #include <QJsonArray>
+#include <QTimer>
 
-ElevationProvider::ElevationProvider(QObject* parent)
-    : QObject(parent)
+QGC_LOGGING_CATEGORY(ElevationProviderLog, "ElevationProviderLog")
+
+Q_GLOBAL_STATIC(TerrainBatchManager, _terrainBatchManager)
+
+TerrainBatchManager::TerrainBatchManager(void)
 {
+    _batchTimer.setSingleShot(true);
+    _batchTimer.setInterval(_batchTimeout);
+    connect(&_batchTimer, &QTimer::timeout, this, &TerrainBatchManager::_sendNextBatch);
+}
 
+void TerrainBatchManager::addQuery(ElevationProvider* elevationProvider, const QList<QGeoCoordinate>& coordinates)
+{
+    if (coordinates.length() > 0) {
+        QueuedRequestInfo_t queuedRequestInfo = { elevationProvider, coordinates };
+        _requestQueue.append(queuedRequestInfo);
+        if (!_batchTimer.isActive()) {
+            _batchTimer.start();
+        }
+    }
 }
 
-bool ElevationProvider::queryTerrainData(const QList<QGeoCoordinate>& coordinates)
+void TerrainBatchManager::_sendNextBatch(void)
 {
-    if (_state != State::Idle || coordinates.length() == 0) {
-        return false;
+    qCDebug(ElevationProviderLog) << "_sendNextBatch _state:_requestQueue.count" << (int)_state << _requestQueue.count();
+
+    if (_state != State::Idle) {
+        // Waiting for last download the complete, wait some more
+        _batchTimer.start();
+        return;
     }
 
-    QUrlQuery query;
-    QString points = "";
-    for (const auto& coordinate : coordinates) {
-        points += QString::number(coordinate.latitude(), 'f', 10) + ","
-                + QString::number(coordinate.longitude(), 'f', 10) + ",";
+    if (_requestQueue.count() == 0) {
+        return;
     }
-    points = points.mid(0, points.length() - 1); // remove the last ','
 
+    _sentRequests.clear();
+
+    // Convert coordinates to point strings for json query
+    QString points;
+    foreach (const QueuedRequestInfo_t& requestInfo, _requestQueue) {
+        SentRequestInfo_t sentRequestInfo = { requestInfo.elevationProvider, requestInfo.coordinates.count() };
+        qCDebug(ElevationProviderLog) << "Building request: coordinate count" << requestInfo.coordinates.count();
+        _sentRequests.append(sentRequestInfo);
+
+        foreach (const QGeoCoordinate& coord, requestInfo.coordinates) {
+            points += QString::number(coord.latitude(), 'f', 10) + ","
+                    + QString::number(coord.longitude(), 'f', 10) + ",";
+        }
+
+    }
+    points = points.mid(0, points.length() - 1); // remove the last ',' from string
+    _requestQueue.clear();
+
+    QUrlQuery query;
     query.addQueryItem(QStringLiteral("points"), points);
     QUrl url(QStringLiteral("https://api.airmap.com/elevation/stage/srtm1/ele"));
     url.setQuery(query);
@@ -50,28 +86,36 @@ bool ElevationProvider::queryTerrainData(const QList<QGeoCoordinate>& coordinate
 
     QNetworkReply* networkReply = _networkManager.get(request);
     if (!networkReply) {
-        return false;
+        _batchFailed();
+        return;
     }
 
-    connect(networkReply, &QNetworkReply::finished, this, &ElevationProvider::_requestFinished);
+    connect(networkReply, &QNetworkReply::finished, this, &TerrainBatchManager::_requestFinished);
 
     _state = State::Downloading;
-    return true;
 }
 
-void ElevationProvider::_requestFinished()
+void TerrainBatchManager::_batchFailed(void)
+{
+    QList<float>    noAltitudes;
+
+    foreach (const SentRequestInfo_t& sentRequestInfo, _sentRequests) {
+        sentRequestInfo.elevationProvider->_signalTerrainData(false, noAltitudes);
+    }
+    _sentRequests.clear();
+}
+
+void TerrainBatchManager::_requestFinished()
 {
+    qCDebug(ElevationProviderLog) << "_requestFinished";
     QNetworkReply* reply = qobject_cast<QNetworkReply*>(QObject::sender());
-    QList<float> altitudes;
+
     _state = State::Idle;
 
     // When an error occurs we still end up here
     if (reply->error() != QNetworkReply::NoError) {
-        QByteArray responseBytes = reply->readAll();
-
-        QJsonParseError parseError;
-        QJsonDocument responseJson = QJsonDocument::fromJson(responseBytes, &parseError);
-        emit terrainData(false, altitudes);
+        _batchFailed();
+        reply->deleteLater();
         return;
     }
 
@@ -80,18 +124,52 @@ void ElevationProvider::_requestFinished()
     QJsonParseError parseError;
     QJsonDocument responseJson = QJsonDocument::fromJson(responseBytes, &parseError);
     if (parseError.error != QJsonParseError::NoError) {
-        emit terrainData(false, altitudes);
+        _batchFailed();
+        reply->deleteLater();
         return;
     }
+
     QJsonObject rootObject = responseJson.object();
     QString status = rootObject["status"].toString();
-    if (status == "success") {
-        const QJsonArray& dataArray = rootObject["data"].toArray();
-        for (int i = 0; i < dataArray.count(); i++) {
-            altitudes.push_back(dataArray[i].toDouble());
-        }
+    if (status != "success") {
+        _batchFailed();
+        reply->deleteLater();
+        return;
+    }
 
-        emit terrainData(true, altitudes);
+    QList<float> altitudes;
+    const QJsonArray& dataArray = rootObject["data"].toArray();
+    for (int i = 0; i < dataArray.count(); i++) {
+        altitudes.push_back(dataArray[i].toDouble());
+    }
+
+    int currentIndex = 0;
+    foreach (const SentRequestInfo_t& sentRequestInfo, _sentRequests) {
+        QList<float> requestAltitudes = altitudes.mid(currentIndex, sentRequestInfo.cCoord);
+        sentRequestInfo.elevationProvider->_signalTerrainData(true, requestAltitudes);
+        currentIndex += sentRequestInfo.cCoord;
     }
-    emit terrainData(false, altitudes);
+    _sentRequests.clear();
+
+    reply->deleteLater();
+}
+
+ElevationProvider::ElevationProvider(QObject* parent)
+    : QObject(parent)
+{
+
+}
+void ElevationProvider::queryTerrainData(const QList<QGeoCoordinate>& coordinates)
+{
+    qCDebug(ElevationProviderLog) << "queryTerrainData: coordinate count" << coordinates.count();
+    if (coordinates.length() == 0) {
+        return;
+    }
+
+    _terrainBatchManager->addQuery(this, coordinates);
+}
+
+void ElevationProvider::_signalTerrainData(bool success, QList<float>& altitudes)
+{
+    emit terrainData(success, altitudes);
 }

src/Terrain.h

@@ -9,47 +9,72 @@
 
 #pragma once
 
+#include "QGCLoggingCategory.h"
+
 #include <QObject>
 #include <QGeoCoordinate>
 #include <QNetworkAccessManager>
+#include <QTimer>
 
-/* usage example:
-    ElevationProvider *p = new ElevationProvider();
-    QList<QGeoCoordinate> coordinates;
-    QGeoCoordinate c(47.379243, 8.548265);
-    coordinates.push_back(c);
-    c.setLatitude(c.latitude()+0.01);
-    coordinates.push_back(c);
-    p->queryTerrainData(coordinates);
- */
+Q_DECLARE_LOGGING_CATEGORY(ElevationProviderLog)
 
+class ElevationProvider;
 
-class ElevationProvider : public QObject
-{
+/// Used internally by ElevationProvider to batch requests together
+class TerrainBatchManager : public QObject {
     Q_OBJECT
-public:
-    ElevationProvider(QObject* parent = NULL);
 
-    /**
-     * Async elevation query for a list of lon,lat coordinates. When the query is done, the terrainData() signal
-     * is emitted.
-     * @param coordinates
-     * @return true on success
-     */
-    bool queryTerrainData(const QList<QGeoCoordinate>& coordinates);
+public:
+    TerrainBatchManager(void);
 
-signals:
-    void terrainData(bool success, QList<float> altitudes);
+    void addQuery(ElevationProvider* elevationProvider, const QList<QGeoCoordinate>& coordinates);
 
 private slots:
-    void _requestFinished();
+    void _sendNextBatch     (void);
+    void _requestFinished   (void);
+
 private:
+    typedef struct {
+        ElevationProvider*      elevationProvider;
+        QList<QGeoCoordinate>   coordinates;
+    } QueuedRequestInfo_t;
+
+    typedef struct {
+        ElevationProvider*      elevationProvider;
+        int                     cCoord;
+    } SentRequestInfo_t;
+
 
     enum class State {
         Idle,
         Downloading,
     };
 
-    State                   _state = State::Idle;
-    QNetworkAccessManager   _networkManager;
+    void _batchFailed(void);
+
+    QList<QueuedRequestInfo_t>  _requestQueue;
+    QList<SentRequestInfo_t>    _sentRequests;
+    State                       _state = State::Idle;
+    QNetworkAccessManager       _networkManager;
+    const int                   _batchTimeout = 500;
+    QTimer                      _batchTimer;
+};
+
+/// NOTE: ElevationProvider is not thread safe. All instances/calls to ElevationProvider must be on main thread.
+class ElevationProvider : public QObject
+{
+    Q_OBJECT
+public:
+    ElevationProvider(QObject* parent = NULL);
+
+     /// Async elevation query for a list of lon,lat coordinates. When the query is done, the terrainData() signal
+     /// is emitted.
+     ///    @param coordinates to query
+    void queryTerrainData(const QList<QGeoCoordinate>& coordinates);
+
+    // Internal method
+    void _signalTerrainData(bool success, QList<float>& altitudes);
+
+signals:
+    void terrainData(bool success, QList<float> altitudes);
 };