Merge pull request #840 from DonLakeFlyer/RCCalFixes

Updated RC Cal validation rules

src/qgcunittest/PX4RCCalibrationTest.h

@@ -30,10 +30,11 @@
 #include "px4_configuration/PX4RCCalibration.h"
 
 /// @file
-///     @brief QGCPX4RCCAlibration Widget unit test
+///     @brief PX4RCCalibration Widget unit test
 ///
 ///     @author Don Gagne <don@thegagnes.com>
 
+///     @brief PX4RCCalibration Widget unit test
 class PX4RCCalibrationTest : public QObject
 {
     Q_OBJECT
@@ -42,6 +43,7 @@ public:
     PX4RCCalibrationTest(void);
     
 private slots:
+    void initTestCase(void);
     void init(void);
     void cleanup(void);
     
@@ -50,7 +52,6 @@ private slots:
     //void _liveRC_test(void);
     void _beginState_test(void);
     void _identifyState_test(void);
-    void _identifyStateSkipOptional_test(void);
     void _minMaxState_test(void);
     void _centerThrottleState_test(void);
     void _detectInversionState_test(void);
@@ -58,13 +59,20 @@ private slots:
     void _fullCalibration_test(void);
     
 private:
-    void _centerAllChannels(void);
-    void _beginState_worker(bool standaloneTest);
-    void _identifyState_worker(bool standaloneTest, bool skipOptional);
-    void _minMaxState_worker(bool standaloneTest);
-    void _centerThrottleState_worker(bool standaloneTest);
-    void _detectInversionState_worker(bool standaloneTest);
-    void _trimsState_worker(bool standaloneTest);
+    /// @brief Modes to run worker functions
+    enum TestMode {
+        testModeStandalone,     ///< Perform standalone test of calibration state
+        testModePrerequisite,   ///< Setup prequisites for subsequent calibration state
+        testModeFullSequence,   ///< Run as full calibration sequence
+    };
+    
+    void _centerChannels(void);
+    void _beginState_worker(enum TestMode mode);
+    void _identifyState_worker(enum TestMode mode);
+    void _minMaxState_worker(enum TestMode mode);
+    void _centerThrottleState_worker(enum TestMode mode);
+    void _detectInversionState_worker(enum TestMode mode);
+    void _trimsState_worker(enum TestMode mode);
     
     enum {
         validateMinMaxMask =    1 << 0,
@@ -73,7 +81,19 @@ private:
         validateMappingMask =   1 << 3,
         validateAllMask = 0xFFFF
     };
-    void _validateParameters(int validateMask, bool skipOptional = false);
+
+    struct ChannelSettings {
+        int rcMin;
+        int rcMax;
+        int rcTrim;
+        int reversed;
+        bool isMinMaxShown;
+        bool isMinValid;
+        bool isMaxValid;
+    };
+    
+    void _validateParameters(int validateMask);
+    void _validateWidgets(int validateMask, const struct ChannelSettings* rgChannelSettings);
     
     MockUASManager*     _mockUASManager;
     MockUAS*            _mockUAS;
@@ -94,6 +114,21 @@ private:
     QLabel*         _statusLabel;
     
     RCChannelWidget* _rgRadioWidget[PX4RCCalibration::_chanMax];
+    
+    // When settings values into min/max/trim we set them slightly different than the defaults so that
+    // we can distinguish between the two values.
+    static const int _testMinValue;
+    static const int _testMaxValue;
+    static const int _testTrimValue;
+    static const int _testThrottleTrimValue;
+    
+    static const int _availableChannels = 8;    ///< 8 channel RC Trasmitter
+    static const int _requiredChannels = 5;     ///< Required channels are 0-4
+    static const int _minMaxChannels = _requiredChannels + 1;    ///< Send min/max to channels 0-5
+    static const int _attitudeChannels = 4;     ///< Attitude channels are 0-3
+    
+    static const struct ChannelSettings _rgChannelSettingsPreValidate[_availableChannels];
+    static const struct ChannelSettings _rgChannelSettingsPostValidate[PX4RCCalibration::_chanMax];
 };
 
 DECLARE_TEST(PX4RCCalibrationTest)

src/ui/designer/RCChannelWidget.cc

@@ -37,6 +37,8 @@ RCChannelWidget::RCChannelWidget(QWidget *parent) :
     _min(_centerValue),
     _max(_centerValue),
     _trim(_centerValue),
+    _minValid(false),
+    _maxValid(false),
     _showMinMax(false),
     _showTrim(false)
 {
@@ -90,19 +92,36 @@ void RCChannelWidget::paintEvent(QPaintEvent *event)
         painter.setBrush(hGradientBrush);
 
         if (_showMinMax) {
+            QString text;
+            
             // Draw the Min numeric value display to the left
             painter.drawText(0, rowHeigth, minMaxDisplayWidth, fontHeight, Qt::AlignHCenter | Qt::AlignBottom, "Min");
-            painter.drawText(0, rowHeigth * 2, minMaxDisplayWidth, fontHeight, Qt::AlignHCenter | Qt::AlignBottom, QString::number(_min));
+            if (_minValid) {
+                text = QString::number(_min);
+            } else {
+                text = "----";
+            }
+            painter.drawText(0, rowHeigth * 2, minMaxDisplayWidth, fontHeight, Qt::AlignHCenter | Qt::AlignBottom, text);
 
             // Draw the Max numeric value display to the right
             painter.drawText(width() - minMaxDisplayWidth, rowHeigth, minMaxDisplayWidth, fontHeight, Qt::AlignHCenter | Qt::AlignBottom, "Max");
-            painter.drawText(width() - minMaxDisplayWidth, rowHeigth * 2, minMaxDisplayWidth, fontHeight, Qt::AlignHCenter | Qt::AlignBottom, QString::number(_max));
+            if (_maxValid) {
+                text = QString::number(_max);
+            } else {
+                text = QString::number(_max);
+            }
+            painter.drawText(width() - minMaxDisplayWidth, rowHeigth * 2, minMaxDisplayWidth, fontHeight, Qt::AlignHCenter | Qt::AlignBottom, text);
             
             // Draw the Min/Max tick marks on the axis
-            int xTick = rcValueAxis.left() + (rcValueAxis.width() * ((float)(_min-_minRange) / (_maxRange-_minRange)));
-            painter.drawLine(xTick, rcValueAxis.top(), xTick, rcValueAxis.bottom());
-            xTick = rcValueAxis.left() + (rcValueAxis.width() * ((float)(_max-_minRange) / (_maxRange-_minRange)));
-            painter.drawLine(xTick, rcValueAxis.top(), xTick, rcValueAxis.bottom());
+            int xTick;
+            if (_minValid) {
+                int xTick = rcValueAxis.left() + (rcValueAxis.width() * ((float)(_min-_minRange) / (_maxRange-_minRange)));
+                painter.drawLine(xTick, rcValueAxis.top(), xTick, rcValueAxis.bottom());
+            }
+            if (_maxValid) {
+                xTick = rcValueAxis.left() + (rcValueAxis.width() * ((float)(_max-_minRange) / (_maxRange-_minRange)));
+                painter.drawLine(xTick, rcValueAxis.top(), xTick, rcValueAxis.bottom());
+            }
         }
         
         if (_showTrim) {
@@ -221,3 +240,17 @@ void RCChannelWidget::_drawValuePointer(QPainter* painter, int xTip, int yTip, i
     
     painter->drawPolygon (trianglePoints, 3);
 }
+
+/// @brief Set whether the Min range value is valid or not.
+void RCChannelWidget::setMinValid(bool valid)
+{
+    _minValid = valid;
+    update();
+}
+
+/// @brief Set whether the Max range value is valid or not.
+void RCChannelWidget::setMaxValid(bool valid)
+{
+    _maxValid = valid;
+    update();
+}

src/ui/designer/RCChannelWidget.h

@@ -46,6 +46,12 @@ public:
     
     void setMinMax(int min, int max);
     
+    /// @brief Set whether the Min range value is valid or not.
+    void setMinValid(bool valid);
+
+    /// @brief Set whether the Max range value is valid or not.
+    void setMaxValid(bool valid);
+    
     /// @brief Sets the Trim value for the channel
     void setTrim(int value);
     
@@ -56,6 +62,8 @@ public:
 
     void showMinMax(bool show);
     bool isMinMaxShown() { return _showMinMax; }
+    bool isMinValid(void) { return _minValid; }
+    bool isMaxValid(void) { return _maxValid; }
     
     void showTrim(bool show);
     bool isTrimShown() { return _showTrim; }
@@ -71,6 +79,9 @@ private:
     int _max;           ///< Max RC value
     int _trim;          ///< RC Value for Trim position
     
+    bool _minValid;     ///< true: minimum value is valid
+    bool _maxValid;     ///< true: maximum value is valid
+    
     bool _showMinMax;   ///< true: show min max values on display
     bool _showTrim;     ///< true: show trim value on display
     

src/ui/px4_configuration/PX4RCCalibration.cc

@@ -31,13 +31,16 @@
 #include "PX4RCCalibration.h"
 #include "UASManager.h"
 
-const int PX4RCCalibration::_updateInterval = 150;          ///< Interval for timer which updates radio channel widgets
-const int PX4RCCalibration::_rcCalPWMValidMinValue = 1000;
-const int PX4RCCalibration::_rcCalPWMValidMaxValue = 2000;
+const int PX4RCCalibration::_updateInterval = 150;              ///< Interval for timer which updates radio channel widgets
 const int PX4RCCalibration::_rcCalPWMCenterPoint = ((PX4RCCalibration::_rcCalPWMValidMaxValue - PX4RCCalibration::_rcCalPWMValidMinValue) / 2.0f) + PX4RCCalibration::_rcCalPWMValidMinValue;
-const int PX4RCCalibration::_rcCalRoughCenterDelta = 200;   ///< Delta around center point which is considered to be roughly centered
-const float PX4RCCalibration::_rcCalMoveDelta = 300.0f;     ///< Amount of delta which is considered stick movement
-const float PX4RCCalibration::_rcCalMinDelta = 100.0f;      ///< Amount of delta allowed around min value to consider channel at min
+const int PX4RCCalibration::_rcCalPWMValidMinValue = 1300;      ///< Largest valid minimum PWM Min range value
+const int PX4RCCalibration::_rcCalPWMValidMaxValue = 1700;      ///< Smallest valid maximum PWM Max range value
+const int PX4RCCalibration::_rcCalPWMDefaultMinValue = 1000;    ///< Default value for Min if not set
+const int PX4RCCalibration::_rcCalPWMDefaultMaxValue = 2000;    ///< Default value for Max if not set
+const int PX4RCCalibration::_rcCalPWMDefaultTrimValue = PX4RCCalibration::_rcCalPWMCenterPoint;    ///< Default value for Trim if not set
+const int PX4RCCalibration::_rcCalRoughCenterDelta = 200;       ///< Delta around center point which is considered to be roughly centered
+const float PX4RCCalibration::_rcCalMoveDelta = 300.0f;         ///< Amount of delta which is considered stick movement
+const float PX4RCCalibration::_rcCalMinDelta = 100.0f;          ///< Amount of delta allowed around min value to consider channel at min
 
 const struct PX4RCCalibration::FunctionInfo PX4RCCalibration::_rgFunctionInfo[PX4RCCalibration::rcCalFunctionMax] = {
     // Name                  Inversion Message   Parameter          required
@@ -274,6 +277,31 @@ void PX4RCCalibration::_setActiveUAS(UASInterface* active)
     setEnabled(_mav ? true : false);
 }
 
+/// @brief Validates the current settings against the calibration rules resetting values as necessary.
+void PX4RCCalibration::_validateCalibration(void)
+{
+    for (int chan = 0; chan<_chanMax; chan++) {
+        struct ChannelInfo* info = &_rgChannelInfo[chan];
+        
+        if (chan < _chanCount) {
+            // Validate Min/Max values. Although the channel appears as available we still may
+            // not have good min/max/trim values for it. Set to defaults if needed.
+            if (info->rcMin > _rcCalPWMValidMinValue || info->rcMax < _rcCalPWMValidMaxValue) {
+                info->rcMin = _rcCalPWMDefaultMinValue;
+                info->rcMax = _rcCalPWMDefaultMaxValue;
+                info->rcTrim = _rcCalPWMDefaultTrimValue;
+            }
+        } else {
+            // Unavailable channels are set to defaults
+            info->rcMin = _rcCalPWMDefaultMinValue;
+            info->rcMax = _rcCalPWMDefaultMaxValue;
+            info->rcTrim = _rcCalPWMDefaultTrimValue;
+            info->reversed = false;
+        }
+    }
+}
+
+
 /// @brief Saves the rc calibration values to the board parameters.
 ///     @param trimsOnly true: write only trim values, false: write all calibration values
 void PX4RCCalibration::_writeCalibration(bool trimsOnly)
@@ -282,6 +310,8 @@ void PX4RCCalibration::_writeCalibration(bool trimsOnly)
     
     _mav->endRadioControlCalibration();
     
+    _validateCalibration();
+    
     QGCUASParamManagerInterface* paramMgr = _mav->getParamManager();
     Q_ASSERT(paramMgr);
     
@@ -290,17 +320,14 @@ void PX4RCCalibration::_writeCalibration(bool trimsOnly)
     QString trimTpl("RC%1_TRIM");
     QString revTpl("RC%1_REV");
     
-    // Do not write the RC type, as these values depend on this
-    // active onboard parameter
-    
-    for (int i = 0; i < _chanCount; ++i) {
-        struct ChannelInfo* info = &_rgChannelInfo[i];
-
-        paramMgr->setPendingParam(0, trimTpl.arg(i+1), info->rcTrim);
+    for (int chan = 0; chan<_chanMax; chan++) {
+        struct ChannelInfo* info = &_rgChannelInfo[chan];
+        int oneBasedChannel = chan + 1;
+        paramMgr->setPendingParam(0, trimTpl.arg(oneBasedChannel), info->rcTrim);
         if (!trimsOnly) {
-            paramMgr->setPendingParam(0, minTpl.arg(i+1), info->rcMin);
-            paramMgr->setPendingParam(0, maxTpl.arg(i+1), info->rcMax);
-            paramMgr->setPendingParam(0, revTpl.arg(i+1), info->reversed ? -1.0f : 1.0f);
+            paramMgr->setPendingParam(0, minTpl.arg(oneBasedChannel), info->rcMin);
+            paramMgr->setPendingParam(0, maxTpl.arg(oneBasedChannel), info->rcMax);
+            paramMgr->setPendingParam(0, revTpl.arg(oneBasedChannel), info->reversed ? -1.0f : 1.0f);
         }
     }
     
@@ -355,13 +382,7 @@ void PX4RCCalibration::_remoteControlChannelRawChanged(int chan, float fval)
         case rcCalStateIdentify:
             if (!_rcCalStateChannelComplete) {
                 // If this channel is already used in a mapping we can't used it again
-                bool channelAlreadyMapped = false;
-                for (int chanFunction=0; chanFunction<rcCalFunctionMax; chanFunction++) {
-                    if (_rgFunctionChannelMapping[chanFunction] == chan) {
-                        channelAlreadyMapped = true;
-                        break;
-                    }
-                }
+                bool channelAlreadyMapped = !(_rgChannelInfo[chan].function == rcCalFunctionMax);
                 
                 // If the channel moved considerably, pick it
                 if (!channelAlreadyMapped && fabsf(_rcValueSave[chan] - fval) > _rcCalMoveDelta) {
@@ -383,10 +404,12 @@ void PX4RCCalibration::_remoteControlChannelRawChanged(int chan, float fval)
             break;
             
         case rcCalStateMinMax:
-            if (fval < _rgChannelInfo[chan].rcMin) {
+            if (fval < _rgChannelInfo[chan].rcMin && fval <= _rcCalPWMValidMinValue) {
+                _rgRadioWidget[chan]->setMinValid(true);
                 _rgChannelInfo[chan].rcMin = fval;
             }
-            if (fval > _rgChannelInfo[chan].rcMax) {
+            if (fval > _rgChannelInfo[chan].rcMax  && fval >= _rcCalPWMValidMaxValue) {
+                _rgRadioWidget[chan]->setMaxValid(true);
                 _rgChannelInfo[chan].rcMax = fval;
             }
             break;
@@ -572,7 +595,11 @@ void PX4RCCalibration::_rcCalChannelWait(bool firstTime)
 {
     _rcCalState = rcCalStateChannelWait;
     
-    _resetInternalCalibrationValues();
+    if (firstTime) {
+        _resetInternalCalibrationValues();
+    } else {
+        _setInternalCalibrationValuesFromParameters();
+    }
 
     if (_chanCount == 0) {
         _ui->rcCalFound->setText(tr("Please turn on Radio"));
@@ -741,18 +768,9 @@ void PX4RCCalibration::_rcCalTrims(void)
     _ui->rcCalSkip->setEnabled(false);
     _ui->rcCalCancel->setEnabled(true);
     
-    _initializeTrims();
     _showTrimOnRadioWidgets(true);
 }
 
-/// @brief Initializes the trim values based on current min/max.
-void PX4RCCalibration::_initializeTrims(void)
-{
-    for (int chan=0; chan<=_chanMax; chan++) {
-        _rgChannelInfo[chan].rcTrim = ((_rgChannelInfo[chan].rcMax - _rgChannelInfo[chan].rcMin) / 2.0f) + _rgChannelInfo[chan].rcMin;
-    }
-}
-
 /// @brief Set up the Save state of calibration.
 void PX4RCCalibration::_rcCalSave(void)
 {
@@ -766,6 +784,12 @@ void PX4RCCalibration::_rcCalSave(void)
     _ui->rcCalTryAgain->setEnabled(false);
     _ui->rcCalSkip->setEnabled(false);
     _ui->rcCalCancel->setEnabled(true);
+    
+    // This updates the internal values according to the validation rules. Then _updateView will tick and update ui
+    // such that the settings that will be written our are displayed.
+    _validateCalibration();
+    
+    _showMinMaxOnRadioWidgets(true);
 }
 
 /// @brief This is used by unit test code to force the calibration state machine to the specified state.
@@ -811,12 +835,26 @@ void PX4RCCalibration::_unitTestForceCalState(enum rcCalStates state) {
 ///     @param show true: show the min/max values, false: hide the min/max values
 void PX4RCCalibration::_showMinMaxOnRadioWidgets(bool show)
 {
+    // Force a view update to widget have current values
+    _updateView();
+    
     // Turn on min/max display for all radio widgets
     for (int i=0; i<_chanMax; i++) {
         RCChannelWidget* radioWidget = _rgRadioWidget[i];
         Q_ASSERT(radioWidget);
         
         radioWidget->showMinMax(show);
+        if (show) {
+            if (radioWidget->min() <= _rcCalPWMValidMinValue) {
+                radioWidget->setMinValid(true);
+            }
+            if (radioWidget->max() >= _rcCalPWMValidMaxValue) {
+                radioWidget->setMaxValid(true);
+            }
+        } else {
+            radioWidget->setMinValid(false);
+            radioWidget->setMaxValid(false);
+        }
     }
 }
 

src/ui/px4_configuration/PX4RCCalibration.h

@@ -124,10 +124,10 @@ private:
     
     // Methods - see source code for documentation
     
+    void _validateCalibration(void);
     void _writeCalibration(bool trimsOnly);
     void _resetInternalCalibrationValues(void);
     void _setInternalCalibrationValuesFromParameters(void);
-    void _initializeTrims(void);
     
     void _rcCalChannelWait(bool firstTime);
     void _rcCalBegin(void);
@@ -164,12 +164,15 @@ private:
     int _rcCalStateIdentifyOldMapping;  ///< Previous mapping for channel being currently identified
     int _rcCalStateReverseOldMapping;   ///< Previous mapping for channel being currently used to detect inversion
     
-    static const int _rcCalPWMCenterPoint;      ///< PWM center value;
-    static const int _rcCalPWMValidMinValue;    ///< Valid minimum PWM value
-    static const int _rcCalPWMValidMaxValue;    ///< Valid maximum PWM value
-    static const int _rcCalRoughCenterDelta;    ///< Delta around center point which is considered to be roughly centered
-    static const float _rcCalMoveDelta;         ///< Amount of delta which is considered stick movement
-    static const float _rcCalMinDelta;          ///< Amount of delta allowed around min value to consider channel at min
+    static const int _rcCalPWMCenterPoint;
+    static const int _rcCalPWMValidMinValue;
+    static const int _rcCalPWMValidMaxValue;
+    static const int _rcCalPWMDefaultMinValue;
+    static const int _rcCalPWMDefaultMaxValue;
+    static const int _rcCalPWMDefaultTrimValue;
+    static const int _rcCalRoughCenterDelta;
+    static const float _rcCalMoveDelta;
+    static const float _rcCalMinDelta;
     
     float _rcValueSave[_chanMax];        ///< Saved values prior to detecting channel movement