Merge pull request #2583 from DonLakeFlyer/MissionClose

Prevent loss of mission due to load or close app

src/MissionEditor/MissionEditor.qml

@@ -40,6 +40,8 @@ import QGroundControl.Controllers   1.0
 QGCView {
     id:         _root
 
+    property bool syncNeeded: controller.missionItems.dirty // Unsaved changes, visible to parent container
+
     viewPanel:          panel
     topDialogMargin:    height - mainWindow.availableHeight
 
@@ -73,7 +75,6 @@ QGCView {
     property var    liveHomePositionAvailable:  controller.liveHomePositionAvailable
     property var    homePosition:               _defaultVehicleCoordinate
 
-    property bool _syncNeeded:                  controller.missionItems.dirty
     property bool _syncInProgress:              _activeVehicle ? _activeVehicle.missionManager.inProgress : false
 
     property bool _showHelp:                    QGroundControl.flightMapSettings.loadBoolMapSetting(editorMap.mapName, _showHelpKey, true)
@@ -458,7 +459,7 @@ QGCView {
                     DropButton {
                         id:                 syncButton
                         dropDirection:      dropRight
-                        buttonImage:        _syncNeeded ? "/qmlimages/MapSyncChanged.svg" : "/qmlimages/MapSync.svg"
+                        buttonImage:        syncNeeded ? "/qmlimages/MapSyncChanged.svg" : "/qmlimages/MapSync.svg"
                         viewportMargins:    ScreenTools.defaultFontPixelWidth / 2
                         exclusiveGroup:     _dropButtonsExclusiveGroup
                         z:                  QGroundControl.zOrderWidgets
@@ -595,6 +596,34 @@ QGCView {
     } // QGCViewPanel
 
     Component {
+        id: syncLoadFromVehicleOverwrite
+
+        QGCViewMessage {
+            id:         syncLoadFromVehicleCheck
+            message:   "Load vehicle check"
+
+            function accept() {
+                hideDialog()
+                controller.getMissionItems()
+            }
+        }
+    }
+
+    Component {
+        id: syncLoadFromFileOverwrite
+
+        QGCViewMessage {
+            id:         syncLoadFromVehicleCheck
+            message:   "Load file check"
+
+            function accept() {
+                hideDialog()
+                controller.loadMissionFromFile()
+            }
+        }
+    }
+
+    Component {
         id: syncDropDownComponent
 
         Column {
@@ -604,7 +633,7 @@ QGCView {
             QGCLabel {
                 width:      columnHolder.width
                 wrapMode:   Text.WordWrap
-                text:       _syncNeeded && !controller.autoSync ?
+                text:       syncNeeded && !controller.autoSync ?
                                 "You have unsaved changed to you mission. You should send to your vehicle, or save to a file:" :
                                 "Sync:"
             }
@@ -629,7 +658,11 @@ QGCView {
 
                     onClicked: {
                         syncButton.hideDropDown()
-                        controller.getMissionItems()
+                        if (syncNeeded) {
+                            _root.showDialog(syncLoadFromVehicleOverwrite, "Mission overwrite", _root.showDialogDefaultWidth, StandardButton.Yes | StandardButton.Cancel)
+                        } else {
+                            controller.getMissionItems()
+                        }
                     }
                 }
             }
@@ -652,7 +685,11 @@ QGCView {
 
                     onClicked: {
                         syncButton.hideDropDown()
-                        controller.loadMissionFromFile()
+                        if (syncNeeded) {
+                            _root.showDialog(syncLoadFromFileOverwrite, "Mission overwrite", _root.showDialogDefaultWidth, StandardButton.Yes | StandardButton.Cancel)
+                        } else {
+                            controller.loadMissionFromFile()
+                        }
                     }
                 }
             }

src/QGCApplication.cc

@@ -692,9 +692,9 @@ void QGCApplication::showSetupView(void)
     QMetaObject::invokeMethod(_rootQmlObject(), "showSetupView");
 }
 
-void QGCApplication::showWindowCloseMessage(void)
+void QGCApplication::qmlAttemptWindowClose(void)
 {
-    QMetaObject::invokeMethod(_rootQmlObject(), "showWindowCloseMessage");
+    QMetaObject::invokeMethod(_rootQmlObject(), "attemptWindowClose");
 }
 
 

src/QGCApplication.h

@@ -135,7 +135,7 @@ public slots:
     void showPlanView(void);
     void showSetupView(void);
 
-    void showWindowCloseMessage(void);
+    void qmlAttemptWindowClose(void);
 
 #ifndef __mobile__
     /// Save the specified Flight Data Log

src/ui/MainWindow.cc

@@ -135,6 +135,7 @@ MainWindow::MainWindow()
     : _lowPowerMode(false)
     , _showStatusBar(false)
     , _mainQmlWidgetHolder(NULL)
+    , _forceClose(false)
 {
     Q_ASSERT(_instance == NULL);
     _instance = this;
@@ -161,6 +162,7 @@ MainWindow::MainWindow()
     _centralLayout->addWidget(_mainQmlWidgetHolder);
     _mainQmlWidgetHolder->setVisible(true);
 
+    QQmlEngine::setObjectOwnership(this, QQmlEngine::CppOwnership);
     _mainQmlWidgetHolder->setContextPropertyObject("controller", this);
     _mainQmlWidgetHolder->setSource(QUrl::fromUserInput("qrc:qml/MainWindowHybrid.qml"));
 
@@ -407,56 +409,29 @@ void MainWindow::showStatusBarCallback(bool checked)
     checked ? statusBar()->show() : statusBar()->hide();
 }
 
-void MainWindow::acceptWindowClose(void)
+void MainWindow::reallyClose(void)
 {
-    qgcApp()->toolbox()->linkManager()->shutdown();
-    // The above shutdown causes a flurry of activity as the vehicle components are removed. This in turn
-    // causes the Windows Version of Qt to crash if you allow the close event to be accepted. In order to prevent
-    // the crash, we ignore the close event and setup a delayed timer to close the window after things settle down.
-    QTimer::singleShot(1500, this, &MainWindow::_closeWindow);
+    _forceClose = true;
+    close();
 }
 
 void MainWindow::closeEvent(QCloseEvent *event)
 {
-    // Disallow window close if there are active connections
-    if (qgcApp()->toolbox()->multiVehicleManager()->vehicles()->count()) {
-        qgcApp()->showWindowCloseMessage();
+    if (!_forceClose) {
+        // Attemp close from within the root Qml item
+        qgcApp()->qmlAttemptWindowClose();
         event->ignore();
         return;
     }
 
-    // We still need to shutdown LinkManager even though no active connections so that we don't get any
-    // more auto-connect links during shutdown.
-    qgcApp()->toolbox()->linkManager()->shutdown();
-
-    // This will process any remaining flight log save dialogs
-    qgcApp()->processEvents(QEventLoop::ExcludeUserInputEvents);
-
     // Should not be any active connections
     if (qgcApp()->toolbox()->linkManager()->anyActiveLinks()) {
         qWarning() << "All links should be disconnected by now";
     }
 
-    // We have to pull out the QmlWidget from the main window and delete it here, before
-    // the MainWindow ends up getting deleted. Otherwise the Qml has a reference to MainWindow
-    // inside it which in turn causes a shutdown crash.
-
-    //-- Unit test gets here with _mainQmlWidgetHolder being NULL
-
-    if(_mainQmlWidgetHolder)
-    {
-        // Remove image provider
-        _mainQmlWidgetHolder->getEngine()->removeImageProvider(QLatin1String("QGCImages"));
-        _centralLayout->removeWidget(_mainQmlWidgetHolder);
-        delete _mainQmlWidgetHolder;
-        _mainQmlWidgetHolder = NULL;
-    }
-
     _storeCurrentViewState();
     storeSettings();
 
-    event->accept();
-
     //-- TODO: This effectively causes the QGCApplication destructor to not being able
     //   to access the pointer it is trying to delete.
     _instance = NULL;

src/ui/MainWindow.h

@@ -96,7 +96,7 @@ public:
     void saveLastUsedConnection(const QString connection);
 
     // Called from MainWindow.qml when the user accepts the window close dialog
-    Q_INVOKABLE void acceptWindowClose(void);
+    Q_INVOKABLE void reallyClose(void);
 
     /// @return Root qml object of main window QML
     QObject* rootQmlObject(void);
@@ -230,6 +230,8 @@ private:
 
     QGCQmlWidgetHolder*     _mainQmlWidgetHolder;
 
+    bool    _forceClose;
+
     QString _getWindowGeometryKey();
 };
 

src/ui/MainWindowHybrid.qml

@@ -25,7 +25,8 @@ import QtQuick          2.5
 import QtQuick.Controls 1.2
 import QtQuick.Dialogs  1.2
 
-import QGroundControl.Controls 1.0
+import QGroundControl           1.0
+import QGroundControl.Controls  1.0
 
 /// Native QML top level window
 Item {
@@ -41,8 +42,8 @@ Item {
         mainWindowInner.item.showSetupView()
     }
 
-    function showWindowCloseMessage() {
-        windowCloseDialog.open()
+    function attemptWindowClose() {
+        mainWindowInner.item.attemptWindowClose()
     }
 
     // The following are use for unit testing only
@@ -71,17 +72,11 @@ Item {
         id:             mainWindowInner
         anchors.fill:   parent
         source:         "MainWindowInner.qml"
-    }
 
-    MessageDialog {
-        id:                 windowCloseDialog
-        title:              "QGroundControl close"
-        text:               "There are still active connections to vehicles. Do you want to disconnect these before closing?"
-        standardButtons:    StandardButton.Yes | StandardButton.Cancel
-        modality:           Qt.ApplicationModal
-        visible:            false
+        Connections {
+            target: mainWindowInner.item
 
-        onYes: controller.acceptWindowClose()
+            onReallyClose: controller.reallyClose()
+        }
     }
 }
-

src/ui/MainWindowInner.qml

@@ -37,6 +37,8 @@ import QGroundControl.MultiVehicleManager   1.0
 Item {
     id:         mainWindow
 
+    signal reallyClose
+
     readonly property string _planViewSource:   "MissionEditor.qml"
     readonly property string _setupViewSource:  "SetupView.qml"
 
@@ -111,6 +113,68 @@ Item {
         setupViewLoader.item.showVehicleComponentPanel(vehicleComponent)
     }
 
+    /// Start the process of closing QGroundControl. Prompts the user are needed.
+    function attemptWindowClose() {
+        unsavedMissionCloseDialog.check()
+    }
+
+    function finishCloseProcess() {
+        QGroundControl.linkManager.shutdown()
+        // The above shutdown causes a flurry of activity as the vehicle components are removed. This in turn
+        // causes the Windows Version of Qt to crash if you allow the close event to be accepted. In order to prevent
+        // the crash, we ignore the close event and setup a delayed timer to close the window after things settle down.
+        delayedWindowCloseTimer.start()
+    }
+
+    MessageDialog {
+        id:                 unsavedMissionCloseDialog
+        title:              "QGroundControl close"
+        text:               "You have a mission edit in progress which has not been saved/sent. If you close you will lose changes. Are you sure you want to close?"
+        standardButtons:    StandardButton.Yes | StandardButton.No
+        modality:           Qt.ApplicationModal
+        visible:            false
+
+        onYes: activeConnectionsCloseDialog.check()
+
+        function check() {
+            if (planViewLoader.item && planViewLoader.item.syncNeeded) {
+                unsavedMissionCloseDialog.open()
+            } else {
+                activeConnectionsCloseDialog.check()
+            }
+        }
+    }
+
+    MessageDialog {
+        id:                 activeConnectionsCloseDialog
+        title:              "QGroundControl close"
+        text:               "There are still active connections to vehicles. Do you want to disconnect these before closing?"
+        standardButtons:    StandardButton.Yes | StandardButton.Cancel
+        modality:           Qt.ApplicationModal
+        visible:            false
+        onYes:              finishCloseProcess()
+
+        function check() {
+            if (QGroundControl.multiVehicleManager.activeVehicle) {
+                activeConnectionsCloseDialog.open()
+            } else {
+                finishCloseProcess()
+            }
+        }
+    }
+
+    Timer {
+        id:         delayedWindowCloseTimer
+        interval:   1500
+        running:    false
+        repeat:     false
+
+        onTriggered: {
+            mainWindow.reallyClose()
+        }
+    }
+
+
     //-- Detect tablet position
     PositionSource {
         id:             positionSource

src/ui/MainWindowNative.qml

@@ -32,17 +32,13 @@ Window {
     id:         _rootWindow
     visible:    true
 
+    property bool _forceClose: false
+
     onClosing: {
-        // Disallow window close if there are active connections
-        if (QGroundControl.multiVehicleManager.activeVehicle) {
-            showWindowCloseMessage()
+        if (!_forceClose) {
+            mainWindowInner.item.attemptWindowClose()
             close.accepted = false
-            return
         }
-
-        // We still need to shutdown LinkManager even though no active connections so that we don't get any
-        // more auto-connect links during shutdown.
-        QGroundControl.linkManager.shutdown();
     }
 
     function showFlyView() {
@@ -57,10 +53,6 @@ Window {
         mainWindowInner.item.showSetupView()
     }
 
-    function showWindowCloseMessage() {
-        windowCloseDialog.open()
-    }
-
     // The following are use for unit testing only
 
     function showSetupFirmware() {
@@ -87,33 +79,15 @@ Window {
         id:             mainWindowInner
         anchors.fill:   parent
         source:         "MainWindowInner.qml"
-    }
 
-    MessageDialog {
-        id:                 windowCloseDialog
-        title:              "QGroundControl close"
-        text:               "There are still active connections to vehicles. Do you want to disconnect these before closing?"
-        standardButtons:    StandardButton.Yes | StandardButton.Cancel
-        modality:           Qt.ApplicationModal
-        visible:            false
-
-        onYes: {
-            QGroundControl.linkManager.shutdown()
-            // The above shutdown causes a flurry of activity as the vehicle components are removed. This in turn
-            // causes the Windows Version of Qt to crash if you allow the close event to be accepted. In order to prevent
-            // the crash, we ignore the close event and setup a delayed timer to close the window after things settle down.
-            delayedWindowCloseTimer.start()
-        }
-    }
-
-    Timer {
-        id:         delayedWindowCloseTimer
-        interval:   1500
-        running:    false
-        repeat:     false
+        Connections {
+            target: mainWindowInner.item
 
-        onTriggered: _rootWindow.close()
+            onReallyClose: {
+                _forceClose = true
+                _rootWindow.close()
+            }
+        }
     }
-
 }