OP-907 fixed regression in vehicle config : "dirty" state management

was broken.
+ added comments

ground/openpilotgcs/src/plugins/config/cfg_vehicletypes/configccpmwidget.cpp

@@ -277,6 +277,41 @@ void ConfigCcpmWidget::setupUI(QString frameType)
     Q_UNUSED(frameType);
 }
 
+void ConfigCcpmWidget::registerWidgets(ConfigTaskWidget &parent) {
+    parent.addWidget(m_aircraft->ccpmType);
+    parent.addWidget(m_aircraft->ccpmTailChannel);
+    parent.addWidget(m_aircraft->ccpmEngineChannel);
+    parent.addWidget(m_aircraft->ccpmServoWChannel);
+    parent.addWidget(m_aircraft->ccpmServoXChannel);
+    parent.addWidget(m_aircraft->ccpmServoYChannel);
+    parent.addWidget(m_aircraft->ccpmSingleServo);
+    parent.addWidget(m_aircraft->ccpmServoZChannel);
+    parent.addWidget(m_aircraft->ccpmAngleW);
+    parent.addWidget(m_aircraft->ccpmAngleX);
+    parent.addWidget(m_aircraft->ccpmCorrectionAngle);
+    parent.addWidget(m_aircraft->ccpmAngleZ);
+    parent.addWidget(m_aircraft->ccpmAngleY);
+    parent.addWidget(m_aircraft->ccpmCollectivePassthrough);
+    parent.addWidget(m_aircraft->ccpmLinkRoll);
+    parent.addWidget(m_aircraft->ccpmLinkCyclic);
+    parent.addWidget(m_aircraft->ccpmRevoSlider);
+    parent.addWidget(m_aircraft->ccpmREVOspinBox);
+    parent.addWidget(m_aircraft->ccpmCollectiveSlider);
+    parent.addWidget(m_aircraft->ccpmCollectivespinBox);
+    parent.addWidget(m_aircraft->ccpmCollectiveScale);
+    parent.addWidget(m_aircraft->ccpmCollectiveScaleBox);
+    parent.addWidget(m_aircraft->ccpmCyclicScale);
+    parent.addWidget(m_aircraft->ccpmPitchScale);
+    parent.addWidget(m_aircraft->ccpmPitchScaleBox);
+    parent.addWidget(m_aircraft->ccpmRollScale);
+    parent.addWidget(m_aircraft->ccpmRollScaleBox);
+    parent.addWidget(m_aircraft->SwashLvlPositionSlider);
+    parent.addWidget(m_aircraft->SwashLvlPositionSpinBox);
+    parent.addWidget(m_aircraft->ThrottleCurve->getCurveWidget());
+    parent.addWidget(m_aircraft->PitchCurve->getCurveWidget());
+    parent.addWidget(m_aircraft->ccpmAdvancedSettingsTable);
+}
+
 void ConfigCcpmWidget::resetActuators(GUIConfigDataUnion *configData)
 {
     configData->heli.Throttle = 0;

ground/openpilotgcs/src/plugins/config/cfg_vehicletypes/configccpmwidget.h

@@ -97,6 +97,7 @@ private:
 
     int MixerChannelData[6];
 
+    virtual void registerWidgets(ConfigTaskWidget &parent);
     virtual void resetActuators(GUIConfigDataUnion *configData);
 
     int ShowDisclaimer(int messageID);

ground/openpilotgcs/src/plugins/config/cfg_vehicletypes/configcustomwidget.cpp

@@ -68,6 +68,7 @@ ConfigCustomWidget::ConfigCustomWidget(QWidget *parent) :
     for (int i = 1; i < (int) VehicleConfig::CHANNEL_NUMELEM; i++) {
         m_aircraft->customMixerTable->setItemDelegateForRow(i, sbd);
     }
+
 }
 
 ConfigCustomWidget::~ConfigCustomWidget()
@@ -80,6 +81,12 @@ void ConfigCustomWidget::setupUI(QString frameType)
     Q_ASSERT(m_aircraft);
 }
 
+void ConfigCustomWidget::registerWidgets(ConfigTaskWidget &parent) {
+    parent.addWidget(m_aircraft->customMixerTable);
+    parent.addWidget(m_aircraft->customThrottle1Curve->getCurveWidget());
+    parent.addWidget(m_aircraft->customThrottle2Curve->getCurveWidget());
+}
+
 void ConfigCustomWidget::resetActuators(GUIConfigDataUnion *configData)
 {
 }

ground/openpilotgcs/src/plugins/config/cfg_vehicletypes/configcustomwidget.h

@@ -61,6 +61,7 @@ protected:
 private:
     Ui_CustomConfigWidget *m_aircraft;
 
+    virtual void registerWidgets(ConfigTaskWidget &parent);
     virtual void resetActuators(GUIConfigDataUnion *configData);
 
 private slots:

ground/openpilotgcs/src/plugins/config/cfg_vehicletypes/configfixedwingwidget.cpp

@@ -165,6 +165,21 @@ void ConfigFixedWingWidget::setupUI(QString frameType)
 	}
 }
 
+void ConfigFixedWingWidget::registerWidgets(ConfigTaskWidget &parent) {
+    parent.addWidget(m_aircraft->fixedWingThrottle->getCurveWidget());
+    parent.addWidget(m_aircraft->fixedWingType);
+
+    parent.addWidget(m_aircraft->fwEngineChannelBox);
+    parent.addWidget(m_aircraft->fwAileron1ChannelBox);
+    parent.addWidget(m_aircraft->fwAileron2ChannelBox);
+    parent.addWidget(m_aircraft->fwElevator1ChannelBox);
+    parent.addWidget(m_aircraft->fwElevator2ChannelBox);
+    parent.addWidget(m_aircraft->fwRudder1ChannelBox);
+    parent.addWidget(m_aircraft->fwRudder2ChannelBox);
+    parent.addWidget(m_aircraft->elevonSlider1);
+    parent.addWidget(m_aircraft->elevonSlider2);
+}
+
 void ConfigFixedWingWidget::resetActuators(GUIConfigDataUnion *configData)
 {
     configData->fixedwing.FixedWingPitch1 = 0;

ground/openpilotgcs/src/plugins/config/cfg_vehicletypes/configfixedwingwidget.h

@@ -57,6 +57,7 @@ public:
 private:
     Ui_FixedWingConfigWidget *m_aircraft;
 
+    virtual void registerWidgets(ConfigTaskWidget &parent);
     virtual void resetActuators(GUIConfigDataUnion *configData);
 
     bool setupFrameFixedWing(QString airframeType);

ground/openpilotgcs/src/plugins/config/cfg_vehicletypes/configgroundvehiclewidget.cpp

@@ -186,6 +186,12 @@ void ConfigGroundVehicleWidget::setupUI(QString frameType)
     }
 }
 
+void ConfigGroundVehicleWidget::registerWidgets(ConfigTaskWidget &parent) {
+    parent.addWidget(m_aircraft->groundVehicleThrottle1->getCurveWidget());
+    parent.addWidget(m_aircraft->groundVehicleThrottle2->getCurveWidget());
+    parent.addWidget(m_aircraft->groundVehicleType);
+}
+
 void ConfigGroundVehicleWidget::resetActuators(GUIConfigDataUnion *configData)
 {
     configData->ground.GroundVehicleSteering1 = 0;

ground/openpilotgcs/src/plugins/config/cfg_vehicletypes/configgroundvehiclewidget.h

@@ -57,6 +57,7 @@ public:
 private:
     Ui_GroundConfigWidget *m_aircraft;
 
+    virtual void registerWidgets(ConfigTaskWidget &parent);
     virtual void resetActuators(GUIConfigDataUnion *configData);
 
     bool setupGroundVehicleCar(QString airframeType);

ground/openpilotgcs/src/plugins/config/cfg_vehicletypes/configmultirotorwidget.cpp

@@ -249,6 +249,23 @@ void ConfigMultiRotorWidget::setupUI(QString frameType)
     updateAirframe(frameType);
 }
 
+void ConfigMultiRotorWidget::registerWidgets(ConfigTaskWidget &parent) {
+    parent.addWidget(m_aircraft->multiThrottleCurve->getCurveWidget());
+    parent.addWidget(m_aircraft->multirotorFrameType);
+    parent.addWidget(m_aircraft->multiMotorChannelBox1);
+    parent.addWidget(m_aircraft->multiMotorChannelBox2);
+    parent.addWidget(m_aircraft->multiMotorChannelBox3);
+    parent.addWidget(m_aircraft->multiMotorChannelBox4);
+    parent.addWidget(m_aircraft->multiMotorChannelBox5);
+    parent.addWidget(m_aircraft->multiMotorChannelBox6);
+    parent.addWidget(m_aircraft->multiMotorChannelBox7);
+    parent.addWidget(m_aircraft->multiMotorChannelBox8);
+    parent.addWidget(m_aircraft->mrPitchMixLevel);
+    parent.addWidget(m_aircraft->mrRollMixLevel);
+    parent.addWidget(m_aircraft->mrYawMixLevel);
+    parent.addWidget(m_aircraft->triYawChannelBox);
+}
+
 void ConfigMultiRotorWidget::resetActuators(GUIConfigDataUnion *configData)
 {
     configData->multi.VTOLMotorN = 0;

ground/openpilotgcs/src/plugins/config/cfg_vehicletypes/configmultirotorwidget.h

@@ -64,6 +64,7 @@ private:
     QGraphicsSvgItem *quad;
     bool invertMotors;
 
+    virtual void registerWidgets(ConfigTaskWidget &parent);
     virtual void resetActuators(GUIConfigDataUnion *configData);
 
     bool setupQuad(bool pLayout);

ground/openpilotgcs/src/plugins/config/cfg_vehicletypes/vehicleconfig.cpp

@@ -105,21 +105,29 @@ void VehicleConfig::setupUI(QString frameType)
     Q_UNUSED(frameType);
 }
 
-QString VehicleConfig::updateConfigObjectsFromWidgets()
+//void VehicleConfig::refreshWidgetsValues(UAVObject *o = NULL)
-{
+//void VehicleConfig::updateObjectsFromWidgets()
-    return QString();
-}
 
 void VehicleConfig::refreshWidgetsValues(QString frameType)
 {
     Q_UNUSED(frameType);
 }
 
+QString VehicleConfig::updateConfigObjectsFromWidgets()
+{
+    return QString();
+}
+
 void VehicleConfig::resetActuators(GUIConfigDataUnion *configData)
 {
     Q_UNUSED(configData);
 }
 
+
+void VehicleConfig::registerWidgets(ConfigTaskWidget &parent) {
+    Q_UNUSED(parent);
+}
+
 // NEW STYLE: Loop through the widgets looking for all widgets that have "ChannelBox" in their name
 // The upshot of this is that ALL new ComboBox widgets for selecting the output channel must have "ChannelBox" in their name
 // FOR WHATEVER REASON, THIS DOES NOT WORK WITH ChannelBox. ChannelBo is sufficiently accurate

ground/openpilotgcs/src/plugins/config/cfg_vehicletypes/vehicleconfig.h

@@ -102,6 +102,14 @@ typedef union
     groundGUISettingsStruct ground;
 } GUIConfigDataUnion;
 
+class ConfigTaskWidget;
+
+/*
+ * This class handles vehicle specific configuration UI and associated logic.
+ *
+ * This class derives from ConfigTaskWidget and overrides its the default "binding" mechanism.
+ * It does not use the "dirty" state management directlyand registers its relevant widgets with ConfigTaskWidget to do so.
+ */
 class VehicleConfig: public ConfigTaskWidget
 {
 Q_OBJECT
@@ -152,6 +160,8 @@ public:
     VehicleConfig(QWidget *parent = 0);
     ~VehicleConfig();
 
+    virtual void registerWidgets(ConfigTaskWidget &parent);
+
     virtual void refreshWidgetsValues(QString frameType);
     virtual QString updateConfigObjectsFromWidgets();
 
@@ -178,6 +188,10 @@ protected:
     double  getCurveMin(QList<double> *curve);
     double  getCurveMax(QList<double> *curve);
 
+//protected slots:
+//    virtual void refreshWidgetsValues(UAVObject *o = NULL);
+//    virtual void updateObjectsFromWidgets();
+
 private:
     static UAVObjectManager *getUAVObjectManager();
 

ground/openpilotgcs/src/plugins/config/configvehicletypewidget.cpp

@@ -142,6 +142,15 @@ ConfigVehicleTypeWidget::ConfigVehicleTypeWidget(QWidget *parent) : ConfigTaskWi
 
     refreshWidgetsValues();
 
+    // register widgets for dirty state management
+    addWidget(m_aircraft->aircraftType);
+
+    // register FF widgets for dirty state management
+    addWidget(m_aircraft->feedForwardSlider);
+    addWidget(m_aircraft->accelTime);
+    addWidget(m_aircraft->decelTime);
+    addWidget(m_aircraft->maxAccelSlider);
+
     disableMouseWheelEvents();
 }
 
@@ -162,19 +171,18 @@ void ConfigVehicleTypeWidget::switchAirframeType(int index)
 
 /**
   Refreshes the current value of the SystemSettings which holds the aircraft type
+  Note: The default behavior of ConfigTaskWidget is bypassed.
+  Therefore no automatic synchronization of UAV Objects to UI is done.
   */
 void ConfigVehicleTypeWidget::refreshWidgetsValues(UAVObject *o)
 {
     Q_UNUSED(o);
 
-    qDebug() << "ConfigVehicleTypeWidget::refreshWidgetsValues - begin";
-
     if (!allObjectsUpdated()) {
         return;
     }
 	
     bool dirty = isDirty();
-    qDebug() << "ConfigVehicleTypeWidget::refreshWidgetsValues - isDirty:" << dirty;
 	
     // Get the Airframe type from the system settings:
     UAVDataObject *system = dynamic_cast<UAVDataObject*>(getObjectManager()->getObject(QString("SystemSettings")));
@@ -192,7 +200,9 @@ void ConfigVehicleTypeWidget::refreshWidgetsValues(UAVObject *o)
     setComboCurrentIndex(m_aircraft->aircraftType, m_aircraft->aircraftType->findText(category));
 
     VehicleConfig *vehicleConfig = getVehicleConfigWidget(category);
+    if (vehicleConfig) {
         vehicleConfig->refreshWidgetsValues(frameType);
+    }
 	
     updateFeedForwardUI();
 
@@ -207,25 +217,19 @@ void ConfigVehicleTypeWidget::refreshWidgetsValues(UAVObject *o)
   We do all the tasks common to all airframes, or family of airframes, and
   we call additional methods for specific frames, so that we do not have a code
   that is too heavy.
+
+  Note: The default behavior of ConfigTaskWidget is bypassed.
+  Therefore no automatic synchronization of UI to UAV Objects is done.
 */
 void ConfigVehicleTypeWidget::updateObjectsFromWidgets()
 {
-    UAVDataObject *mixer = dynamic_cast<UAVDataObject *>(getObjectManager()->getObject(QString("MixerSettings")));
+    // Airframe type defaults to Custom
-    Q_ASSERT(mixer);
-
-    QPointer<VehicleConfig> vconfig = new VehicleConfig();
-
-    // Update feed forward settings
-    vconfig->setMixerValue(mixer, "FeedForward", m_aircraft->feedForwardSlider->value() / 100.0);
-    vconfig->setMixerValue(mixer, "AccelTime", m_aircraft->accelTime->value());
-    vconfig->setMixerValue(mixer, "DecelTime", m_aircraft->decelTime->value());
-    vconfig->setMixerValue(mixer, "MaxAccel", m_aircraft->maxAccelSlider->value());
-
-    // Sets airframe type default to "Custom"
     QString airframeType = "Custom";
 
     VehicleConfig *vehicleConfig = (VehicleConfig *) m_aircraft->airframesWidget->currentWidget();
+    if (vehicleConfig) {
         airframeType = vehicleConfig->updateConfigObjectsFromWidgets();
+    }
 
     // set the airframe type
     UAVDataObject *system = dynamic_cast<UAVDataObject *>(getObjectManager()->getObject(QString("SystemSettings")));
@@ -236,19 +240,21 @@ void ConfigVehicleTypeWidget::updateObjectsFromWidgets()
         field->setValue(airframeType);
     }
 
+    // Update feed forward settings
+    UAVDataObject *mixer = dynamic_cast<UAVDataObject *>(getObjectManager()->getObject(QString("MixerSettings")));
+    Q_ASSERT(mixer);
+
+    QPointer<VehicleConfig> vconfig = new VehicleConfig();
+
+    vconfig->setMixerValue(mixer, "FeedForward", m_aircraft->feedForwardSlider->value() / 100.0);
+    vconfig->setMixerValue(mixer, "AccelTime", m_aircraft->accelTime->value());
+    vconfig->setMixerValue(mixer, "DecelTime", m_aircraft->decelTime->value());
+    vconfig->setMixerValue(mixer, "MaxAccel", m_aircraft->maxAccelSlider->value());
+
+    // TODO call refreshWidgetsValues() to reflect actual saved values ?
     updateFeedForwardUI();
 }
 
-/**
-  Reset the contents of a field
-  */
-//void ConfigVehicleTypeWidget::resetField(UAVObjectField *field)
-//{
-//    for (unsigned int i = 0; i < field->getNumElements(); i++) {
-//        field->setValue(0, i);
-//    }
-//}
-
 QString ConfigVehicleTypeWidget::frameCategory(QString frameType)
 {
     QString category;
@@ -277,20 +283,24 @@ QString ConfigVehicleTypeWidget::frameCategory(QString frameType)
 VehicleConfig *ConfigVehicleTypeWidget::getVehicleConfigWidget(QString frameCategory)
 {
     VehicleConfig *vehiculeConfig;
-    if (vehicleIndexMap.contains(frameCategory)) {
+    if (!vehicleIndexMap.contains(frameCategory)) {
-        int index = vehicleIndexMap.value(frameCategory);
+        // create config widget
-        vehiculeConfig = (VehicleConfig *) m_aircraft->airframesWidget->widget(index);
-    } else {
         vehiculeConfig = createVehicleConfigWidget(frameCategory);
+        // bind config widget "field" to this ConfigTaskWodget
+        // this is necessary to get "dirty" state management
+        vehiculeConfig->registerWidgets(*this);
+        // add config widget to UI
         int index = m_aircraft->airframesWidget->insertWidget(m_aircraft->airframesWidget->count(), vehiculeConfig);
         vehicleIndexMap[frameCategory] = index;
     }
+    int index = vehicleIndexMap.value(frameCategory);
+    vehiculeConfig = (VehicleConfig *) m_aircraft->airframesWidget->widget(index);
     return vehiculeConfig;
 }
 
 VehicleConfig *ConfigVehicleTypeWidget::createVehicleConfigWidget(QString frameCategory)
 {
-    qDebug() << "creating" << frameCategory;
+    qDebug() << "ConfigVehicleTypeWidget::createVehicleConfigWidget - creating" << frameCategory;
     if (frameCategory == "Fixed Wing") {
         return new ConfigFixedWingWidget();
     } else if (frameCategory == "Multirotor") {
@@ -318,7 +328,7 @@ void ConfigVehicleTypeWidget::enableFFTest()
             && m_aircraft->ffTestBox3->isChecked()) {
         if (!ffTuningInProgress) {
             // Initiate tuning:
-            UAVDataObject* obj = dynamic_cast<UAVDataObject*>(getObjectManager()->getObject(
+            UAVDataObject *obj = dynamic_cast<UAVDataObject*>(getObjectManager()->getObject(
                     QString("ManualControlCommand")));
             UAVObject::Metadata mdata = obj->getMetadata();
             accInitialData = mdata;
@@ -356,7 +366,7 @@ void ConfigVehicleTypeWidget::enableFFTest()
         // Disarm!
         if (ffTuningInProgress) {
             ffTuningInProgress = false;
-            UAVDataObject* obj = dynamic_cast<UAVDataObject*>(getObjectManager()->getObject(
+            UAVDataObject *obj = dynamic_cast<UAVDataObject*>(getObjectManager()->getObject(
                     QString("ManualControlCommand")));
             UAVObject::Metadata mdata = obj->getMetadata();
             mdata = accInitialData; // Restore metadata

ground/openpilotgcs/src/plugins/config/configvehicletypewidget.h

@@ -38,6 +38,21 @@
 #include <QStringList>
 #include <QWidget>
 
+/*
+ * This class derives from ConfigTaskWidget and overrides its default "binding" mechanism.
+ * This widget bypasses automatic synchronization of UAVObjects and UI by providing its own implementations of
+ *     virtual void refreshWidgetsValues(UAVObject *obj = NULL);
+ *     virtual void updateObjectsFromWidgets();
+ *
+ * It does use the "dirty" state management and registers its relevant widgets with ConfigTaskWidget to do so.
+ *
+ * This class also manages child ConfigTaskWidget : see VehicleConfig class and its derived classes.
+ * Note: for "dirty" state management it is important to register the fields of child widgets with the parent
+ * ConfigVehicleTypeWidget class.
+ *
+ * TODO consider to call "super" to benefit from default logic...
+ * TODO improve handling of relationship with VehicleConfig derived classes (i.e. ConfigTaskWidget within ConfigTaskWidget)
+ */
 class ConfigVehicleTypeWidget: public ConfigTaskWidget
 {
     Q_OBJECT
@@ -49,7 +64,7 @@ public:
     ConfigVehicleTypeWidget(QWidget *parent = 0);
     ~ConfigVehicleTypeWidget();
 
-public slots:
+protected slots:
     virtual void refreshWidgetsValues(UAVObject *o = NULL);
     virtual void updateObjectsFromWidgets();