OP-1016 GCS Options dialog - fixed issue that allowed a user to delete all configurations of a given category and would lead to a GCS crash

ground/openpilotgcs/src/plugins/coreplugin/dialogs/ioptionspage.h

@@ -39,8 +39,10 @@ class QWidget;
 QT_END_NAMESPACE
 
 namespace Core {
+
 class CORE_EXPORT IOptionsPage : public QObject {
     Q_OBJECT
+
 public:
     IOptionsPage(QObject *parent = 0) :
         QObject(parent),
@@ -51,6 +53,7 @@ public:
     {
         m_icon = icon;
     }
+
     QIcon icon()
     {
         return m_icon;
@@ -63,25 +66,36 @@ public:
     {
         return "";
     };
+
     virtual QString trName() const
     {
         return "";
     };
+
     virtual QString category() const
     {
         return "DefaultCategory";
     };
+
     virtual QString trCategory() const
     {
         return "DefaultCategory";
     };
 
     virtual QWidget *createPage(QWidget *parent) = 0;
+
     virtual void apply()  = 0;
     virtual void finish() = 0;
+
+public slots:
+    virtual void updateState()
+    {
+    };
+
 private:
     QIcon m_icon;
 };
+
 } // namespace Core
 
 #endif // IOPTIONSPAGE_H

ground/openpilotgcs/src/plugins/coreplugin/dialogs/settingsdialog.cpp

@@ -322,6 +322,9 @@ void SettingsDialog::onItemSelected()
         stackedPages->insertWidget(index, page->createPage(stackedPages));
     }
 
+    IOptionsPage *page = m_pages.at(index);
+    page->updateState();
+
     stackedPages->setCurrentIndex(index);
 }
 
@@ -465,10 +468,11 @@ void SettingsDialog::done(int val)
 
     settings->setValue("LastPreferenceCategory", m_currentCategory);
     settings->setValue("LastPreferencePage", m_currentPage);
+
     settings->setValue("WindowWidth", this->width());
     settings->setValue("WindowHeight", this->height());
+
     QList<int> sizes = splitter->sizes();
-    qDebug() << "SettingsDialog splitter saving size0:" << sizes[0] << ", size1:" << sizes[1];
     settings->setValue("SplitterPosition", sizes[0]);
 
     settings->endGroup();

ground/openpilotgcs/src/plugins/coreplugin/uavgadgetinstancemanager.cpp

@@ -303,23 +303,31 @@ void UAVGadgetInstanceManager::removeAllGadgets()
 }
 
 
-bool UAVGadgetInstanceManager::canDeleteConfiguration(IUAVGadgetConfiguration *config)
+bool UAVGadgetInstanceManager::isConfigurationActive(IUAVGadgetConfiguration *config)
 {
-    // to be able to delete a configuration, no instance must be using it
+    // check if there is gadget currently using the configuration
     foreach(IUAVGadget * gadget, m_gadgetInstances) {
         if (gadget->activeConfiguration() == config) {
-            return false;
-        }
-    }
-    // and it cannot be the only configuration
-    foreach(IUAVGadgetConfiguration * c, m_configurations) {
-        if (c != config && c->classId() == config->classId()) {
             return true;
         }
     }
     return false;
 }
 
+UAVGadgetInstanceManager::DeleteStatus UAVGadgetInstanceManager::canDeleteConfiguration(IUAVGadgetConfiguration *config)
+{
+    // to be able to delete a configuration, no instance must be using it
+    if (isConfigurationActive(config)) {
+        return UAVGadgetInstanceManager::KO_ACTIVE;
+    }
+    // and it cannot be the only configuration
+    QList<IUAVGadgetConfiguration *> *configs = provisionalConfigurations(config->classId());
+    if (configs->count() <= 1) {
+        return UAVGadgetInstanceManager::KO_LONE;
+    }
+    return UAVGadgetInstanceManager::OK;
+}
+
 void UAVGadgetInstanceManager::deleteConfiguration(IUAVGadgetConfiguration *config)
 {
     m_provisionalDeletes.append(config);
@@ -504,6 +512,30 @@ QList<IUAVGadgetConfiguration *> *UAVGadgetInstanceManager::configurations(QStri
     return configs;
 }
 
+QList<IUAVGadgetConfiguration *> *UAVGadgetInstanceManager::provisionalConfigurations(QString classId) const
+{
+    QList<IUAVGadgetConfiguration *> *configs = new QList<IUAVGadgetConfiguration *>;
+    // add current configurations
+    foreach(IUAVGadgetConfiguration * config, m_configurations) {
+        if (config->classId() == classId) {
+            configs->append(config);
+        }
+    }
+    // add provisional configurations
+    foreach(IUAVGadgetConfiguration * config, m_provisionalConfigs) {
+        if (config->classId() == classId) {
+            configs->append(config);
+        }
+    }
+    // remove provisional configurations
+    foreach(IUAVGadgetConfiguration * config, m_provisionalDeletes) {
+        if (config->classId() == classId) {
+            configs->removeOne(config);
+        }
+    }
+    return configs;
+}
+
 int UAVGadgetInstanceManager::indexForConfig(QList<IUAVGadgetConfiguration *> configurations,
                                              QString classId, QString configName)
 {

ground/openpilotgcs/src/plugins/coreplugin/uavgadgetinstancemanager.h

@@ -41,6 +41,7 @@ class PluginManager;
 }
 
 namespace Core {
+
 namespace Internal {
 class SettingsDialog;
 }
@@ -53,22 +54,30 @@ class IUAVGadgetFactory;
 class CORE_EXPORT UAVGadgetInstanceManager : public QObject {
     Q_OBJECT
 public:
+    enum DeleteStatus { OK, KO_ACTIVE, KO_LONE };
+
     explicit UAVGadgetInstanceManager(QObject *parent = 0);
     ~UAVGadgetInstanceManager();
+
     void readSettings(QSettings *qs);
     void saveSettings(QSettings *qs);
+
     IUAVGadget *createGadget(QString classId, QWidget *parent, bool loadDefaultConfiguration = true);
     void removeGadget(IUAVGadget *gadget);
     void removeAllGadgets();
-    bool canDeleteConfiguration(IUAVGadgetConfiguration *config);
+
+    bool isConfigurationActive(IUAVGadgetConfiguration *config);
+    DeleteStatus canDeleteConfiguration(IUAVGadgetConfiguration *config);
     void deleteConfiguration(IUAVGadgetConfiguration *config);
     void cloneConfiguration(IUAVGadgetConfiguration *config);
     void applyChanges(IUAVGadgetConfiguration *config);
     void configurationNameEdited(QString text, bool hasText = true);
+
     QStringList classIds() const
     {
         return m_classIdNameMap.keys();
     }
+
     QStringList configurationNames(QString classId) const;
     QString gadgetName(QString classId) const;
     QIcon gadgetIcon(QString classId) const;
@@ -84,10 +93,6 @@ public slots:
     void settingsDialogRemoved();
 
 private:
-    IUAVGadgetFactory *factory(QString classId) const;
-    void createOptionsPages();
-    QList<IUAVGadgetConfiguration *> *configurations(QString classId) const;
-    QString suggestName(QString classId, QString name);
     QList<IUAVGadget *> m_gadgetInstances;
     QList<IUAVGadgetFactory *> m_factories;
     QList<IUAVGadgetConfiguration *> m_configurations;
@@ -100,11 +105,22 @@ private:
     QList<IOptionsPage *> m_provisionalOptionsPages;
     Core::Internal::SettingsDialog *m_settingsDialog;
     ExtensionSystem::PluginManager *m_pm;
-    int indexForConfig(QList<IUAVGadgetConfiguration *> configurations,
-                       QString classId, QString configName);
+
+    IUAVGadgetFactory *factory(QString classId) const;
+
+    void createOptionsPages();
+
+    QList<IUAVGadgetConfiguration *> *configurations(QString classId) const;
+    QList<IUAVGadgetConfiguration *> *provisionalConfigurations(QString classId) const;
+
+    QString suggestName(QString classId, QString name);
+
+    int indexForConfig(QList<IUAVGadgetConfiguration *> configurations, QString classId, QString configName);
+
     void readConfigs_1_1_0(QSettings *qs);
     void readConfigs_1_2_0(QSettings *qs);
 };
+
 } // namespace Core
 
 #endif // UAVGADGETINSTANCEMANAGER_H

ground/openpilotgcs/src/plugins/coreplugin/uavgadgetoptionspagedecorator.cpp

@@ -53,16 +53,6 @@ QWidget *UAVGadgetOptionsPageDecorator::createPage(QWidget *parent)
     m_page = new Ui_TopOptionsPage();
     QWidget *w = new QWidget(parent);
     m_page->setupUi(w);
-    if (m_config->locked()) {
-        m_page->deleteButton->hide();
-        m_page->lockCheckBox->hide();
-        m_page->nameLineEdit->setDisabled(true);
-    }
-    if (!m_instanceManager->canDeleteConfiguration(m_config)) {
-        m_page->deleteButton->setDisabled(true);
-    }
-    m_page->lockCheckBox->hide(); //
-    m_page->nameLineEdit->setText(m_id);
 
     QWidget *wi = m_optionsPage->createPage(w);
     wi->setSizePolicy(QSizePolicy::MinimumExpanding, QSizePolicy::MinimumExpanding);
@@ -73,6 +63,8 @@ QWidget *UAVGadgetOptionsPageDecorator::createPage(QWidget *parent)
         m_page->configurationBox->hide();
     }
 
+    updateState();
+
     connect(m_page->cloneButton, SIGNAL(clicked()), this, SLOT(cloneConfiguration()));
     connect(m_page->deleteButton, SIGNAL(clicked()), this, SLOT(deleteConfiguration()));
     connect(m_page->nameLineEdit, SIGNAL(textEdited(QString)), this, SLOT(textEdited(QString)));
@@ -92,6 +84,35 @@ void UAVGadgetOptionsPageDecorator::finish()
     m_optionsPage->finish();
 }
 
+void UAVGadgetOptionsPageDecorator::updateState()
+{
+    if (m_config->locked()) {
+        m_page->deleteButton->hide();
+        m_page->lockCheckBox->hide();
+        m_page->nameLineEdit->setDisabled(true);
+    }
+    switch(m_instanceManager->canDeleteConfiguration(m_config)) {
+    case UAVGadgetInstanceManager::OK:
+        m_page->deleteButton->setEnabled(true);
+        m_page->deleteButton->setToolTip(tr("Delete this configuration"));
+        break;
+    case UAVGadgetInstanceManager::KO_ACTIVE:
+        m_page->deleteButton->setEnabled(false);
+        m_page->deleteButton->setToolTip(tr("Cannot delete a currently in use configuration"));
+        break;
+    case UAVGadgetInstanceManager::KO_LONE:
+        m_page->deleteButton->setEnabled(false);
+        m_page->deleteButton->setToolTip(tr("Cannot delete the last configuration"));
+        break;
+    default:
+        m_page->deleteButton->setEnabled(false);
+        m_page->deleteButton->setToolTip(tr("DON'T KNOW !"));
+        break;
+    }
+    m_page->lockCheckBox->hide();
+    m_page->nameLineEdit->setText(m_id);
+}
+
 void UAVGadgetOptionsPageDecorator::cloneConfiguration()
 {
     m_instanceManager->cloneConfiguration(m_config);

ground/openpilotgcs/src/plugins/coreplugin/uavgadgetoptionspagedecorator.h

@@ -67,6 +67,7 @@ public:
 signals:
 
 public slots:
+    void updateState();
 
 private slots:
     void cloneConfiguration();