Merge pull request #6825 from DonLakeFlyer/SimpleItemFix

SimpleMissionItem::setCoordinate was screwing up altitude/param7 syncronization

ChangeLog.md

@@ -5,5 +5,7 @@ Note: This file only contains high level features or important fixes.
 ## 3.4
 
 ### 3.4.1
+* Fix bug where new mission items may end up with 0 altitude internally and sent to vehicle while UI shows correct altitude.
 * Fix crash when Survery with terrain follow is moved quickly
-* Fix terrain follow climb/descent rate fields swapped in ui
\ No newline at end of file
+* Fix terrain follow climb/descent rate fields swapped in ui
+

src/MissionManager/MissionItem.cc

@@ -409,13 +409,6 @@ void MissionItem::setParam7(double param)
     }
 }
 
-void MissionItem::setCoordinate(const QGeoCoordinate& coordinate)
-{
-    setParam5(coordinate.latitude());
-    setParam6(coordinate.longitude());
-    setParam7(coordinate.altitude());
-}
-
 QGeoCoordinate MissionItem::coordinate(void) const
 {
     return QGeoCoordinate(param5(), param6(), param7());

src/MissionManager/MissionItem.h

@@ -96,7 +96,6 @@ public:
     void setParam5          (double param5);
     void setParam6          (double param6);
     void setParam7          (double param7);
-    void setCoordinate      (const QGeoCoordinate& coordinate);
     
     void save(QJsonObject& json) const;
     bool load(QTextStream &loadStream);

src/MissionManager/MissionManagerTest.cc

@@ -37,7 +37,9 @@ void MissionManagerTest::_writeItems(MockLinkMissionItemHandler::FailureMode_t f
     // Editor has a home position item on the front, so we do the same
     MissionItem* homeItem = new MissionItem(NULL /* Vehicle */, this);
     homeItem->setCommand(MAV_CMD_NAV_WAYPOINT);
-    homeItem->setCoordinate(QGeoCoordinate(47.3769, 8.549444, 0));
+    homeItem->setParam5(47.3769);
+    homeItem->setParam6(8.549444);
+    homeItem->setParam7(0);
     homeItem->setSequenceNumber(0);
     missionItems.append(homeItem);
 

src/MissionManager/SimpleMissionItem.cc

@@ -815,8 +815,10 @@ void SimpleMissionItem::setCommand(int command)
 
 void SimpleMissionItem::setCoordinate(const QGeoCoordinate& coordinate)
 {
-    if (_missionItem.coordinate() != coordinate) {
-        _missionItem.setCoordinate(coordinate);
+    // We only use lat/lon from coordinate. This keeps param7 and the altitude value which is kept to the side in sync.
+    if (_missionItem.param5() != coordinate.latitude() || _missionItem.param6() != coordinate.longitude()) {
+        _missionItem.setParam5(coordinate.latitude());
+        _missionItem.setParam6(coordinate.longitude());
     }
 }
 

src/MissionManager/SimpleMissionItemTest.cc

@@ -194,8 +194,11 @@ void SimpleMissionItemTest::_testSignals(void)
     _simpleItem->setCoordinate(QGeoCoordinate(missionItem.param5(), missionItem.param6() + 1, missionItem.param7()));
     QVERIFY(_spyVisualItem->checkOnlySignalByMask(coordinateChangedMask | dirtyChangedMask));
     _spyVisualItem->clearAllSignals();
+
+    // Altitude in coordinate is not used in setCoordinate
     _simpleItem->setCoordinate(QGeoCoordinate(missionItem.param5(), missionItem.param6(), missionItem.param7() + 1));
-    QVERIFY(_spyVisualItem->checkOnlySignalByMask(coordinateChangedMask | dirtyChangedMask));
+    QVERIFY(_spyVisualItem->checkNoSignals());
+    QVERIFY(_spySimpleItem->checkNoSignals());
     _spyVisualItem->clearAllSignals();
 
     // Check dirtyChanged signalling