From a045b10472a7800deb5db68938a9dfc783f7fc02 Mon Sep 17 00:00:00 2001 From: Philippe Renon Date: Tue, 16 Sep 2014 08:42:36 +0200 Subject: [PATCH 1/3] OP-1481 fixed telemetry multi-threading issue: was not possible to create and request an uavo instance in one go --- .../openpilotgcs/src/plugins/uavtalk/telemetry.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/ground/openpilotgcs/src/plugins/uavtalk/telemetry.cpp b/ground/openpilotgcs/src/plugins/uavtalk/telemetry.cpp index 059d988b1..08c81e9e9 100644 --- a/ground/openpilotgcs/src/plugins/uavtalk/telemetry.cpp +++ b/ground/openpilotgcs/src/plugins/uavtalk/telemetry.cpp @@ -48,10 +48,12 @@ Telemetry::Telemetry(UAVTalk *utalk, UAVObjectManager *objMngr) : objMngr(objMng } // Listen to new object creations - connect(objMngr, SIGNAL(newObject(UAVObject *)), this, SLOT(newObject(UAVObject *))); - connect(objMngr, SIGNAL(newInstance(UAVObject *)), this, SLOT(newInstance(UAVObject *))); + // connection must be direct, if not, it is not possible to create and send (or request) an object in one go + connect(objMngr, SIGNAL(newObject(UAVObject *)), this, SLOT(newObject(UAVObject *)), Qt::DirectConnection); + connect(objMngr, SIGNAL(newInstance(UAVObject *)), this, SLOT(newInstance(UAVObject *)), Qt::DirectConnection); // Listen to transaction completions + // these slots will be executed in the telemetry thread // TODO should send a status (SUCCESS, FAILED, TIMEOUT) connect(utalk, SIGNAL(transactionCompleted(UAVObject *, bool)), this, SLOT(transactionCompleted(UAVObject *, bool))); @@ -568,6 +570,10 @@ void Telemetry::newObject(UAVObject *obj) { QMutexLocker locker(mutex); +#ifdef VERBOSE_TELEMETRY + qDebug() << "Telemetry - new object" << obj->toStringBrief(); +#endif + registerObject(obj); } @@ -575,6 +581,10 @@ void Telemetry::newInstance(UAVObject *obj) { QMutexLocker locker(mutex); +#ifdef VERBOSE_TELEMETRY + qDebug() << "Telemetry - new object instance" << obj->toStringBrief(); +#endif + registerObject(obj); } From 92e37799979cbec059ebbfd0ec961f1031b9553d Mon Sep 17 00:00:00 2001 From: Philippe Renon Date: Tue, 16 Sep 2014 08:44:58 +0200 Subject: [PATCH 2/3] OP-1481 specified default timeout for AbstractUAVObjectHelper --- .../openpilotgcs/src/plugins/uavobjectutil/uavobjecthelper.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ground/openpilotgcs/src/plugins/uavobjectutil/uavobjecthelper.h b/ground/openpilotgcs/src/plugins/uavobjectutil/uavobjecthelper.h index c7ec495fc..14ecf7df3 100644 --- a/ground/openpilotgcs/src/plugins/uavobjectutil/uavobjecthelper.h +++ b/ground/openpilotgcs/src/plugins/uavobjectutil/uavobjecthelper.h @@ -43,7 +43,10 @@ public: virtual ~AbstractUAVObjectHelper(); enum Result { SUCCESS, FAIL, TIMEOUT }; - Result doObjectAndWait(UAVObject *object, int timeout); + + // default timeout = 3 x 250ms + 50ms safety margin = 800ms + // where 3 is the number of UAVTalk retries and 250ms is the UAVTalk timeout + Result doObjectAndWait(UAVObject *object, int timeout = 800); protected: virtual void doObjectAndWaitImpl() = 0; From 3c4d754e2fe15b767704d8a4951a0e2cb108d9fa Mon Sep 17 00:00:00 2001 From: Philippe Renon Date: Tue, 16 Sep 2014 09:10:36 +0200 Subject: [PATCH 3/3] OP-1481 send and request path plans in user space instead of relying on magic (but broken) sendAll and requestAll uavo methods --- .../src/plugins/opmap/modeluavoproxy.cpp | 157 +++++++++++------- .../src/plugins/opmap/modeluavoproxy.h | 7 - 2 files changed, 97 insertions(+), 67 deletions(-) diff --git a/ground/openpilotgcs/src/plugins/opmap/modeluavoproxy.cpp b/ground/openpilotgcs/src/plugins/opmap/modeluavoproxy.cpp index e328bf0db..229eb608c 100644 --- a/ground/openpilotgcs/src/plugins/opmap/modeluavoproxy.cpp +++ b/ground/openpilotgcs/src/plugins/opmap/modeluavoproxy.cpp @@ -26,7 +26,9 @@ */ #include "modeluavoproxy.h" #include "extensionsystem/pluginmanager.h" +#include "uavobjecthelper.h" +#include #include ModelUavoProxy::ModelUavoProxy(QObject *parent, flightDataModel *model) : QObject(parent), myModel(model) @@ -37,91 +39,126 @@ ModelUavoProxy::ModelUavoProxy(QObject *parent, flightDataModel *model) : QObjec objMngr = pm->getObject(); Q_ASSERT(objMngr != NULL); - - completionCountdown = 0; - successCountdown = 0; } void ModelUavoProxy::sendPathPlan() { modelToObjects(); - PathPlan *pathPlan = PathPlan::GetInstance(objMngr, 0); - connect(pathPlan, SIGNAL(transactionCompleted(UAVObject *, bool)), - this, SLOT(pathPlanElementSent(UAVObject *, bool)), Qt::UniqueConnection); + PathPlan *pathPlan = PathPlan::GetInstance(objMngr); - Waypoint *waypoint = Waypoint::GetInstance(objMngr, 0); - connect(waypoint, SIGNAL(transactionCompleted(UAVObject *, bool)), - this, SLOT(pathPlanElementSent(UAVObject *, bool)), Qt::UniqueConnection); + const int waypointCount = pathPlan->getWaypointCount(); + const int actionCount = pathPlan->getPathActionCount(); - PathAction *action = PathAction::GetInstance(objMngr, 0); - connect(action, SIGNAL(transactionCompleted(UAVObject *, bool)), - this, SLOT(pathPlanElementSent(UAVObject *, bool)), Qt::UniqueConnection); + QProgressDialog progress(tr("Sending the path plan to the board... "), "", 0, 1 + waypointCount + actionCount); + progress.setWindowModality(Qt::WindowModal); + progress.setCancelButton(NULL); + progress.show(); - // we will start 3 update all - completionCountdown = 3; - successCountdown = completionCountdown; + UAVObjectUpdaterHelper updateHelper; - pathPlan->updated(); - waypoint->updatedAll(); - action->updatedAll(); -} + // send PathPlan + bool success = (updateHelper.doObjectAndWait(pathPlan) == UAVObjectUpdaterHelper::SUCCESS); + progress.setValue(1); -void ModelUavoProxy::pathPlanElementSent(UAVObject *obj, bool success) -{ - obj->disconnect(this); - - completionCountdown--; - successCountdown -= success ? 1 : 0; - - if (completionCountdown == 0) { - qDebug() << "ModelUavoProxy::pathPlanSent - completed" << (successCountdown == 0); - if (successCountdown == 0) { - QMessageBox::information(NULL, tr("Path Plan Upload Successful"), tr("Path plan upload was successful.")); - } else { - QMessageBox::critical(NULL, tr("Path Plan Upload Failed"), tr("Failed to upload the path plan !")); + if (success) { + // send Waypoint instances + qDebug() << "sending" << waypointCount << "waypoints"; + for (int i = 0; i < waypointCount; ++i) { + Waypoint *waypoint = Waypoint::GetInstance(objMngr, i); + success = (updateHelper.doObjectAndWait(waypoint) == UAVObjectUpdaterHelper::SUCCESS); + if (!success) { + break; + } + progress.setValue(progress.value() + 1); } } + + if (success) { + // send PathAction instances + qDebug() << "sending" << actionCount << "path actions"; + for (int i = 0; i < actionCount; ++i) { + PathAction *action = PathAction::GetInstance(objMngr, i); + success = (updateHelper.doObjectAndWait(action) == UAVObjectUpdaterHelper::SUCCESS); + if (!success) { + break; + } + progress.setValue(progress.value() + 1); + } + } + + qDebug() << "ModelUavoProxy::pathPlanSent - completed" << success; + if (!success) { + QMessageBox::critical(NULL, tr("Sending Path Plan Failed!"), tr("Failed to send the path plan to the board.")); + } + + progress.close(); } void ModelUavoProxy::receivePathPlan() { - PathPlan *pathPlan = PathPlan::GetInstance(objMngr, 0); + QProgressDialog progress(tr("Receiving the path plan from the board... "), "", 0, 0); + progress.setWindowModality(Qt::WindowModal); + progress.setCancelButton(NULL); + progress.show(); - connect(pathPlan, SIGNAL(transactionCompleted(UAVObject *, bool)), this, SLOT(pathPlanElementReceived(UAVObject *, bool))); + UAVObjectRequestHelper requestHelper; - Waypoint *waypoint = Waypoint::GetInstance(objMngr, 0); - connect(waypoint, SIGNAL(transactionCompleted(UAVObject *, bool)), this, SLOT(pathPlanElementReceived(UAVObject *, bool))); + PathPlan *pathPlan = PathPlan::GetInstance(objMngr); + bool success = (requestHelper.doObjectAndWait(pathPlan) == UAVObjectUpdaterHelper::SUCCESS); - PathAction *action = PathAction::GetInstance(objMngr, 0); - connect(action, SIGNAL(transactionCompleted(UAVObject *, bool)), this, SLOT(pathPlanElementReceived(UAVObject *, bool))); + const int waypointCount = pathPlan->getWaypointCount(); + const int actionCount = pathPlan->getPathActionCount(); - // we will start 3 update requests - completionCountdown = 3; - successCountdown = completionCountdown; + progress.setMaximum(1 + waypointCount + actionCount); + progress.setValue(1); - pathPlan->requestUpdate(); - waypoint->requestUpdateAll(); - action->requestUpdateAll(); -} - -void ModelUavoProxy::pathPlanElementReceived(UAVObject *obj, bool success) -{ - obj->disconnect(this); - - completionCountdown--; - successCountdown -= success ? 1 : 0; - - if (completionCountdown == 0) { - qDebug() << "ModelUavoProxy::pathPlanReceived - completed" << (successCountdown == 0); - if (successCountdown == 0) { - if (objectsToModel()) { - QMessageBox::information(NULL, tr("Path Plan Download Successful"), tr("Path plan download was successful.")); + if (success && (waypointCount > objMngr->getNumInstances(Waypoint::OBJID))) { + // allocate needed Waypoint instances + Waypoint *waypoint = new Waypoint; + waypoint->initialize(waypointCount - 1, waypoint->getMetaObject()); + success = objMngr->registerObject(waypoint); + } + if (success) { + // request Waypoint instances + qDebug() << "requesting" << waypointCount << "waypoints"; + for (int i = 0; i < waypointCount; ++i) { + Waypoint *waypoint = Waypoint::GetInstance(objMngr, i); + success = (requestHelper.doObjectAndWait(waypoint) == UAVObjectRequestHelper::SUCCESS); + if (!success) { + break; } - } else { - QMessageBox::critical(NULL, tr("Path Plan Download Failed"), tr("Failed to download the path plan !")); + progress.setValue(progress.value() + 1); } } + + if (success && (actionCount > objMngr->getNumInstances(PathAction::OBJID))) { + // allocate needed PathAction instances + PathAction *action = new PathAction; + action->initialize(actionCount - 1, action->getMetaObject()); + success = objMngr->registerObject(action); + } + if (success) { + // request PathAction isntances + qDebug() << "requesting" << actionCount << "path actions"; + for (int i = 0; i < actionCount; ++i) { + PathAction *action = PathAction::GetInstance(objMngr, i); + success = (requestHelper.doObjectAndWait(action) == UAVObjectRequestHelper::SUCCESS); + if (!success) { + break; + } + progress.setValue(progress.value() + 1); + } + } + + qDebug() << "ModelUavoProxy::pathPlanReceived - completed" << success; + if (success) { + objectsToModel(); + } else { + QMessageBox::critical(NULL, tr("Receiving Path Plan Failed!"), tr("Failed to receive the path plan from the board.")); + } + + progress.close(); } // update waypoint and path actions UAV objects diff --git a/ground/openpilotgcs/src/plugins/opmap/modeluavoproxy.h b/ground/openpilotgcs/src/plugins/opmap/modeluavoproxy.h index e1f26262f..7adb5560d 100644 --- a/ground/openpilotgcs/src/plugins/opmap/modeluavoproxy.h +++ b/ground/openpilotgcs/src/plugins/opmap/modeluavoproxy.h @@ -49,9 +49,6 @@ private: UAVObjectManager *objMngr; flightDataModel *myModel; - uint completionCountdown; - uint successCountdown; - bool modelToObjects(); bool objectsToModel(); @@ -67,10 +64,6 @@ private: void pathActionToModel(int i, PathAction::DataFields &data); quint8 computePathPlanCrc(int waypointCount, int actionCount); - -private slots: - void pathPlanElementSent(UAVObject *, bool success); - void pathPlanElementReceived(UAVObject *, bool success); }; #endif // MODELUAVOPROXY_H