Commit 5350467e authored by Don Gagne's avatar Don Gagne Committed by GitHub

Merge pull request #3618 from DonLakeFlyer/ParamLoadFixes

Parameter loader fixes
parents 53a49fdc ca438f88
......@@ -571,6 +571,7 @@ HEADERS += \
src/qgcunittest/MavlinkLogTest.h \
src/qgcunittest/MessageBoxTest.h \
src/qgcunittest/MultiSignalSpy.h \
src/qgcunittest/ParameterLoaderTest.h \
src/qgcunittest/RadioConfigTest.h \
src/qgcunittest/TCPLinkTest.h \
src/qgcunittest/TCPLoopBackServer.h \
......@@ -596,6 +597,7 @@ SOURCES += \
src/qgcunittest/MavlinkLogTest.cc \
src/qgcunittest/MessageBoxTest.cc \
src/qgcunittest/MultiSignalSpy.cc \
src/qgcunittest/ParameterLoaderTest.cc \
src/qgcunittest/RadioConfigTest.cc \
src/qgcunittest/TCPLinkTest.cc \
src/qgcunittest/TCPLoopBackServer.cc \
......
......@@ -32,7 +32,7 @@ FactPanel {
TextInput {
text: fact2.value
property Fact fact2: controller.getParameterFact(51, "COMPONENT_51")
property Fact fact2: controller.getParameterFact(50, "RC_MAP_THROTTLE")
onAccepted: fact2.value = text
}
......
......@@ -62,18 +62,7 @@ void FactSystemTestBase::_parameter_specific_component_id_test(void)
QVERIFY(fact != NULL);
QVariant factValue = fact->rawValue();
QCOMPARE(factValue.isValid(), true);
QCOMPARE(factValue.toInt(), 3);
// Test another component id
QVERIFY(_plugin->factExists(FactSystem::ParameterProvider, 51, "COMPONENT_51"));
fact = _plugin->getFact(FactSystem::ParameterProvider, 51, "COMPONENT_51");
QVERIFY(fact != NULL);
factValue = fact->rawValue();
QCOMPARE(factValue.isValid(), true);
QCOMPARE(factValue.toInt(), 51);
}
/// Test that QML can reference a Fact
......
......@@ -47,6 +47,9 @@ ParameterLoader::ParameterLoader(Vehicle* vehicle)
, _defaultComponentId(MAV_COMP_ID_ALL)
, _parameterSetMajorVersion(-1)
, _parameterMetaData(NULL)
, _prevWaitingReadParamIndexCount(0)
, _prevWaitingReadParamNameCount(0)
, _prevWaitingWriteParamNameCount(0)
, _initialRequestRetryCount(0)
, _totalParamCount(0)
{
......@@ -90,14 +93,26 @@ void ParameterLoader::_parameterUpdate(int uasId, int componentId, QString param
_initialRequestTimeoutTimer.stop();
qCDebug(ParameterLoaderLog) << "_parameterUpdate (usaId:" << uasId <<
"componentId:" << componentId <<
"name:" << parameterName <<
"count:" << parameterCount <<
"index:" << parameterId <<
"mavType:" << mavType <<
"value:" << value <<
")";
if (_initialLoadComplete) {
qCDebug(ParameterLoaderLog) << "_parameterUpdate (id:" << uasId <<
"componentId:" << componentId <<
"name:" << parameterName <<
"count:" << parameterCount <<
"index:" << parameterId <<
"mavType:" << mavType <<
"value:" << value <<
")";
} else {
// This is too noisy during initial load
qCDebug(ParameterLoaderVerboseLog) << "_parameterUpdate (id:" << uasId <<
"componentId:" << componentId <<
"name:" << parameterName <<
"count:" << parameterCount <<
"index:" << parameterId <<
"mavType:" << mavType <<
"value:" << value <<
")";
}
#if 0
// Handy for testing retry logic
......@@ -115,7 +130,7 @@ void ParameterLoader::_parameterUpdate(int uasId, int componentId, QString param
}
#endif
if (parameterName == "_HASH_CHECK") {
if (_vehicle->px4Firmware() && parameterName == "_HASH_CHECK") {
/* we received a cache hash, potentially load from cache */
_tryCacheHashLoad(uasId, componentId, value);
return;
......@@ -201,20 +216,25 @@ void ParameterLoader::_parameterUpdate(int uasId, int componentId, QString param
qCDebug(ParameterLoaderLog) << "waitingWriteParamNameCount:" << waitingWriteParamNameCount;
}
int waitingParamCount = waitingReadParamIndexCount + waitingReadParamNameCount + waitingWriteParamNameCount;
if (waitingParamCount) {
qCDebug(ParameterLoaderLog) << "waitingParamCount:" << waitingParamCount;
int readWaitingParamCount = waitingReadParamIndexCount + waitingReadParamNameCount;
int totalWaitingParamCount = readWaitingParamCount + waitingWriteParamNameCount;
if (totalWaitingParamCount) {
qCDebug(ParameterLoaderLog) << "totalWaitingParamCount:" << totalWaitingParamCount;
} else if (_defaultComponentId != MAV_COMP_ID_ALL) {
// No more parameters to wait for, stop the timeout. Be careful to not stop timer if we don't have the default
// component yet.
_waitingParamTimeoutTimer.stop();
}
// Update progress bar
if (waitingParamCount == 0) {
emit parameterListProgress(0);
// Update progress bar for waiting reads
if (readWaitingParamCount == 0) {
// We are no longer waiting for any reads to complete
if (_prevWaitingReadParamIndexCount + _prevWaitingReadParamNameCount != 0) {
// Set progress to 0 if not already there
emit parameterListProgress(0);
}
} else {
emit parameterListProgress((float)(_totalParamCount - waitingParamCount) / (float)_totalParamCount);
emit parameterListProgress((float)(_totalParamCount - readWaitingParamCount) / (float)_totalParamCount);
}
// Get parameter set version
......@@ -286,12 +306,25 @@ void ParameterLoader::_parameterUpdate(int uasId, int componentId, QString param
_setupGroupMap();
}
if (waitingParamCount == 0) {
// Now that we know vehicle is up to date persist
if (_prevWaitingWriteParamNameCount != 0 && waitingWriteParamNameCount == 0) {
// If all the writes just finished the vehicle is up to date, so persist.
_saveToEEPROM();
_writeLocalParamCache(uasId, componentId);
}
// Update param cache. The param cache is only used on PX4 Firmware since ArduPilot and Solo have volatile params
// which invalidate the cache. The Solo also streams param updates in flight for things like gimbal values
// which in turn causes a perf problem with all the param cache updates.
if (_vehicle->px4Firmware()) {
if (_prevWaitingReadParamIndexCount + _prevWaitingReadParamNameCount != 0 && readWaitingParamCount == 0) {
// All reads just finished, update the cache
_writeLocalParamCache(uasId, componentId);
}
}
_prevWaitingReadParamIndexCount = waitingReadParamIndexCount;
_prevWaitingReadParamNameCount = waitingReadParamNameCount;
_prevWaitingWriteParamNameCount = waitingWriteParamNameCount;
// Don't fail initial load complete if default component isn't found yet. That will be handled in wait timeout check.
_checkInitialLoadComplete(false /* failIfNoDefaultComponent */);
}
......@@ -955,8 +988,10 @@ void ParameterLoader::_checkInitialLoadComplete(bool failIfNoDefaultComponent)
"This will cause QGroundControl to be unable to display its full user interface. "
"If you are using modified firmware, you may need to resolve any vehicle startup errors to resolve the issue. "
"If you are using standard firmware, you may need to upgrade to a newer version to resolve the issue.");
qCWarning(ParameterLoaderLog) << "The following parameter indices could not be loaded after the maximum number of retries: " << indexList;
emit parametersReady(true);
if (!qgcApp()->runningUnitTests()) {
qCWarning(ParameterLoaderLog) << "The following parameter indices could not be loaded after the maximum number of retries: " << indexList;
}
emit parametersReady(true /* missingParameters */);
return;
}
......@@ -966,15 +1001,17 @@ void ParameterLoader::_checkInitialLoadComplete(bool failIfNoDefaultComponent)
"This will cause QGroundControl to be unable to display its full user interface. "
"If you are using modified firmware, you may need to resolve any vehicle startup errors to resolve the issue. "
"If you are using standard firmware, you may need to upgrade to a newer version to resolve the issue.");
qCWarning(ParameterLoaderLog) << "Default component was never found, param:" << _defaultComponentIdParam;
emit parametersReady(true);
if (!qgcApp()->runningUnitTests()) {
qCWarning(ParameterLoaderLog) << "Default component was never found, param:" << _defaultComponentIdParam;
}
emit parametersReady(true /* missingParameters */);
return;
}
// No failures, signal good load
_parametersReady = true;
_determineDefaultComponentId();
emit parametersReady(false);
emit parametersReady(false /* no missingParameters */);
}
void ParameterLoader::_initialRequestTimeout(void)
......
......@@ -151,7 +151,13 @@ private:
int _parameterSetMajorVersion; ///< Version for parameter set, -1 if not known
QObject* _parameterMetaData; ///< Opaque data from FirmwarePlugin::loadParameterMetaDataCall
static const int _maxInitialRequestListRetry = 5; ///< Maximum retries for request list
// Wait counts from previous parameter update cycle
int _prevWaitingReadParamIndexCount;
int _prevWaitingReadParamNameCount;
int _prevWaitingWriteParamNameCount;
static const int _maxInitialRequestListRetry = 4; ///< Maximum retries for request list
int _initialRequestRetryCount; ///< Current retry count for request list
static const int _maxInitialLoadRetrySingleParam = 10; ///< Maximum retries for initial index based load of a single param
static const int _maxReadWriteRetry = 5; ///< Maximum retries read/write
......
This diff is collapsed.
......@@ -51,6 +51,15 @@ public:
bool sendStatusText(void) { return _sendStatusText; }
void setSendStatusText(bool sendStatusText) { _sendStatusText = sendStatusText; emit sendStatusChanged(); }
typedef enum {
FailNone, // No failures
FailParamNoReponseToRequestList, // Do no respond to PARAM_REQUEST_LIST
FailMissingParamOnInitialReqest, // Not all params are sent on initial request, should still succeed since QGC will re-query missing params
FailMissingParamOnAllRequests, // Not all params are sent on initial request, QGC retries will fail as well
} FailureMode_t;
FailureMode_t failureMode(void) { return _failureMode; }
void setFailureMode(FailureMode_t failureMode) { _failureMode = failureMode; }
// Overrides from LinkConfiguration
LinkType type (void) { return LinkConfiguration::TypeMock; }
void copyFrom (LinkConfiguration* source);
......@@ -68,10 +77,12 @@ private:
MAV_AUTOPILOT _firmwareType;
MAV_TYPE _vehicleType;
bool _sendStatusText;
FailureMode_t _failureMode;
static const char* _firmwareTypeKey;
static const char* _vehicleTypeKey;
static const char* _sendStatusTextKey;
static const char* _failureModeKey;
};
class MockLink : public LinkInterface
......@@ -88,6 +99,7 @@ public:
MAV_AUTOPILOT getFirmwareType(void) { return _firmwareType; }
void setFirmwareType(MAV_AUTOPILOT autopilot) { _firmwareType = autopilot; }
void setSendStatusText(bool sendStatusText) { _sendStatusText = sendStatusText; }
void setFailureMode(MockConfiguration::FailureMode_t failureMode) { _failureMode = failureMode; }
/// APM stack has strange handling of the first item of the mission list. If it has no
/// onboard mission items, sometimes it sends back a home position in position 0 and
......@@ -132,10 +144,10 @@ public:
/// Reset the state of the MissionItemHandler to no items, no transactions in progress.
void resetMissionItemHandler(void) { _missionItemHandler.reset(); }
static MockLink* startPX4MockLink (bool sendStatusText);
static MockLink* startGenericMockLink (bool sendStatusText);
static MockLink* startAPMArduCopterMockLink (bool sendStatusText);
static MockLink* startAPMArduPlaneMockLink (bool sendStatusText);
static MockLink* startPX4MockLink (bool sendStatusText, MockConfiguration::FailureMode_t failureMode = MockConfiguration::FailNone);
static MockLink* startGenericMockLink (bool sendStatusText, MockConfiguration::FailureMode_t failureMode = MockConfiguration::FailNone);
static MockLink* startAPMArduCopterMockLink (bool sendStatusText, MockConfiguration::FailureMode_t failureMode = MockConfiguration::FailNone);
static MockLink* startAPMArduPlaneMockLink (bool sendStatusText, MockConfiguration::FailureMode_t failureMode = MockConfiguration::FailNone);
private slots:
virtual void _writeBytes(const QByteArray bytes);
......@@ -143,7 +155,7 @@ private slots:
private slots:
void _run1HzTasks(void);
void _run10HzTasks(void);
void _run50HzTasks(void);
void _run500HzTasks(void);
private:
// From LinkInterface
......@@ -175,6 +187,7 @@ private:
void _sendStatusTextMessages(void);
void _respondWithAutopilotVersion(void);
void _sendRCChannels(void);
void _paramRequestListWorker(void);
static MockLink* _startMockLink(MockConfiguration* mockConfig);
......@@ -204,14 +217,19 @@ private:
bool _sendStatusText;
bool _apmSendHomePositionOnEmptyList;
MockConfiguration::FailureMode_t _failureMode;
int _sendHomePositionDelayCount;
int _sendGPSPositionDelayCount;
static float _vehicleLatitude;
static float _vehicleLongitude;
static float _vehicleAltitude;
static int _nextVehicleSystemId;
int _currentParamRequestListComponentIndex; // Current component index for param request list workflow, -1 for no request in progress
int _currentParamRequestListParamIndex; // Current parameter index for param request list workflor
static float _vehicleLatitude;
static float _vehicleLongitude;
static float _vehicleAltitude;
static int _nextVehicleSystemId;
static const char* _failParam;
};
#endif
......@@ -7,10 +7,10 @@
1 50 ATT_J33 0.0037 9
1 50 ATT_J_EN 0 6
1 50 ATT_MAG_DECL 0 9
1 50 BAT_CRIT_THR 0.05 0.09
1 50 BAT_CRIT_THR 0.05 9
1 50 BAT_CAPACITY -1 9
1 50 BAT_C_SCALING 0.0124 9
1 50 BAT_LOW_THR 0.15 0.18
1 50 BAT_LOW_THR 0.15 9
1 50 BAT_N_CELLS 3 6
1 50 BAT_V_CHARGED 4.2 9
1 50 BAT_V_EMPTY 3.4 9
......@@ -109,7 +109,7 @@
1 50 COM_FLTMODE4 -1 6
1 50 COM_FLTMODE5 -1 6
1 50 COM_FLTMODE6 -1 6
1 50 COM_LOW_BAT_ACT 0 0
1 50 COM_LOW_BAT_ACT 0 5
1 50 COM_RC_IN_MODE 0 6
1 50 COM_RC_LOSS_T 0.5 9
1 50 EKF_ATT_V3_Q0 0.0001 9
......
/****************************************************************************
*
* (c) 2009-2016 QGROUNDCONTROL PROJECT <http://www.qgroundcontrol.org>
*
* QGroundControl is licensed according to the terms in the file
* COPYING.md in the root of the source code directory.
*
****************************************************************************/
#include "ParameterLoaderTest.h"
#include "MultiVehicleManager.h"
#include "QGCApplication.h"
#include "ParameterLoader.h"
/// Test failure modes which should still lead to param load success
void ParameterLoaderTest::_noFailureWorker(MockConfiguration::FailureMode_t failureMode)
{
Q_ASSERT(!_mockLink);
_mockLink = MockLink::startPX4MockLink(false, failureMode);
MultiVehicleManager* vehicleMgr = qgcApp()->toolbox()->multiVehicleManager();
QVERIFY(vehicleMgr);
// Wait for the Vehicle to get created
QSignalSpy spyVehicle(vehicleMgr, SIGNAL(activeVehicleAvailableChanged(bool)));
QCOMPARE(spyVehicle.wait(5000), true);
QCOMPARE(spyVehicle.count(), 1);
QList<QVariant> arguments = spyVehicle.takeFirst();
QCOMPARE(arguments.count(), 1);
QCOMPARE(arguments.at(0).toBool(), true);
Vehicle* vehicle = vehicleMgr->activeVehicle();
QVERIFY(vehicle);
// We should get progress bar updates during load
QSignalSpy spyProgress(vehicle->getParameterLoader(), SIGNAL(parameterListProgress(float)));
QCOMPARE(spyProgress.wait(2000), true);
arguments = spyProgress.takeFirst();
QCOMPARE(arguments.count(), 1);
QVERIFY(arguments.at(0).toFloat() > 0.0f);
// When param load is complete we get the param ready signal
QSignalSpy spyParamsReady(vehicleMgr, SIGNAL(parameterReadyVehicleAvailableChanged(bool)));
QCOMPARE(spyParamsReady.wait(60000), true);
arguments = spyParamsReady.takeFirst();
QCOMPARE(arguments.count(), 1);
QCOMPARE(arguments.at(0).toBool(), true);
// Progress should have been set back to 0
arguments = spyProgress.takeLast();
QCOMPARE(arguments.count(), 1);
QCOMPARE(arguments.at(0).toFloat(), 0.0f);
}
void ParameterLoaderTest::_noFailure(void)
{
_noFailureWorker(MockConfiguration::FailNone);
}
void ParameterLoaderTest::_requestListMissingParamSuccess(void)
{
_noFailureWorker(MockConfiguration::FailMissingParamOnInitialReqest);
}
// Test no response to param_request_list
void ParameterLoaderTest::_requestListNoResponse(void)
{
Q_ASSERT(!_mockLink);
_mockLink = MockLink::startPX4MockLink(false, MockConfiguration::FailParamNoReponseToRequestList);
MultiVehicleManager* vehicleMgr = qgcApp()->toolbox()->multiVehicleManager();
QVERIFY(vehicleMgr);
// Wait for the Vehicle to get created
QSignalSpy spyVehicle(vehicleMgr, SIGNAL(activeVehicleAvailableChanged(bool)));
QCOMPARE(spyVehicle.wait(5000), true);
QCOMPARE(spyVehicle.count(), 1);
QList<QVariant> arguments = spyVehicle.takeFirst();
QCOMPARE(arguments.count(), 1);
QCOMPARE(arguments.at(0).toBool(), true);
Vehicle* vehicle = vehicleMgr->activeVehicle();
QVERIFY(vehicle);
QSignalSpy spyParamsReady(vehicleMgr, SIGNAL(parameterReadyVehicleAvailableChanged(bool)));
QSignalSpy spyProgress(vehicle->getParameterLoader(), SIGNAL(parameterListProgress(float)));
// We should not get any progress bar updates, nor a parameter ready signal
QCOMPARE(spyProgress.wait(500), false);
QCOMPARE(spyParamsReady.wait(40000), false);
// User should have been notified
checkMultipleExpectedMessageBox(5);
}
// MockLink will fail to send a param on initial request, it will also fail to send it on subsequent
// param_read requests.
void ParameterLoaderTest::_requestListMissingParamFail(void)
{
// Will pop error about missing params
setExpectedMessageBox(QMessageBox::Ok);
Q_ASSERT(!_mockLink);
_mockLink = MockLink::startPX4MockLink(false, MockConfiguration::FailMissingParamOnAllRequests);
MultiVehicleManager* vehicleMgr = qgcApp()->toolbox()->multiVehicleManager();
QVERIFY(vehicleMgr);
// Wait for the Vehicle to get created
QSignalSpy spyVehicle(vehicleMgr, SIGNAL(activeVehicleAvailableChanged(bool)));
QCOMPARE(spyVehicle.wait(5000), true);
QCOMPARE(spyVehicle.count(), 1);
QList<QVariant> arguments = spyVehicle.takeFirst();
QCOMPARE(arguments.count(), 1);
QCOMPARE(arguments.at(0).toBool(), true);
Vehicle* vehicle = vehicleMgr->activeVehicle();
QVERIFY(vehicle);
QSignalSpy spyParamsReady(vehicleMgr, SIGNAL(parameterReadyVehicleAvailableChanged(bool)));
QSignalSpy spyProgress(vehicle->getParameterLoader(), SIGNAL(parameterListProgress(float)));
// We will get progress bar updates, since it will fail after getting partially through the request
QCOMPARE(spyProgress.wait(2000), true);
arguments = spyProgress.takeFirst();
QCOMPARE(arguments.count(), 1);
QVERIFY(arguments.at(0).toFloat() > 0.0f);
// We should get a parameters ready signal, but Vehicle should indicate missing params
QCOMPARE(spyParamsReady.wait(40000), true);
QCOMPARE(vehicle->missingParameters(), true);
// User should have been notified
checkExpectedMessageBox();
}
/****************************************************************************
*
* (c) 2009-2016 QGROUNDCONTROL PROJECT <http://www.qgroundcontrol.org>
*
* QGroundControl is licensed according to the terms in the file
* COPYING.md in the root of the source code directory.
*
****************************************************************************/
#ifndef ParameterLoaderTest_H
#define ParameterLoaderTest_H
#include "UnitTest.h"
#include "MockLink.h"
#include "MultiSignalSpy.h"
#include "MockLink.h"
class ParameterLoaderTest : public UnitTest
{
Q_OBJECT
private slots:
void _noFailure(void);
void _requestListNoResponse(void);
void _requestListMissingParamSuccess(void);
void _requestListMissingParamFail(void);
private:
void _noFailureWorker(MockConfiguration::FailureMode_t failureMode);
};
#endif
......@@ -198,6 +198,13 @@ void UnitTest::checkExpectedMessageBox(int expectFailFlags)
QCOMPARE(messageBoxRespondedTo, true);
}
void UnitTest::checkMultipleExpectedMessageBox(int messageCount)
{
int missedMessageBoxCount = _missedMessageBoxCount;
_missedMessageBoxCount = 0;
QCOMPARE(missedMessageBoxCount, messageCount);
}
void UnitTest::checkExpectedFileDialog(int expectFailFlags)
{
// Internal testing
......@@ -377,7 +384,7 @@ void UnitTest::_connectMockLink(MAV_AUTOPILOT autopilot)
// Wait for the Vehicle to get created
QSignalSpy spyVehicle(qgcApp()->toolbox()->multiVehicleManager(), SIGNAL(parameterReadyVehicleAvailableChanged(bool)));
QCOMPARE(spyVehicle.wait(5000), true);
QCOMPARE(spyVehicle.wait(10000), true);
QVERIFY(qgcApp()->toolbox()->multiVehicleManager()->parameterReadyVehicleAvailable());
_vehicle = qgcApp()->toolbox()->multiVehicleManager()->activeVehicle();
QVERIFY(_vehicle);
......
......@@ -74,6 +74,9 @@ public:
// @param Expected failure response flags
void checkExpectedMessageBox(int expectFailFlags = expectFailNoFailure);
/// Checks that the specified number of message boxes where shown. Do not call setExpectedMessageBox when using this method.
void checkMultipleExpectedMessageBox(int messageCount);
/// @brief Check whether a message box was displayed and correctly responded to
// @param Expected failure response flags
void checkExpectedFileDialog(int expectFailFlags = expectFailNoFailure);
......
......@@ -29,6 +29,7 @@
#include "MainWindowTest.h"
#include "FileManagerTest.h"
#include "TCPLinkTest.h"
#include "ParameterLoaderTest.h"
UT_REGISTER_TEST(FactSystemTestGeneric)
UT_REGISTER_TEST(FactSystemTestPX4)
......@@ -46,6 +47,7 @@ UT_REGISTER_TEST(MissionManagerTest)
UT_REGISTER_TEST(RadioConfigTest)
UT_REGISTER_TEST(TCPLinkTest)
UT_REGISTER_TEST(FileManagerTest)
UT_REGISTER_TEST(ParameterLoaderTest)
// List of unit test which are currently disabled.
// If disabling a new test, include reason in comment.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment