diff --git a/src/test/test_task_model.cpp b/src/test/test_task_model.cpp index 33635625..2f5617da 100644 --- a/src/test/test_task_model.cpp +++ b/src/test/test_task_model.cpp @@ -43,6 +43,7 @@ #include #include #include +#include using namespace moveit::task_constructor; @@ -200,3 +201,18 @@ TEST_F(TaskListModelTest, visitedPopulate) { EXPECT_EQ(num_inserts, 3); EXPECT_EQ(num_updates, 0); } + +TEST_F(TaskListModelTest, deletion) { + children = 3; + model.processTaskMessage(genMsg("first")); + moveit_rviz_plugin::BaseTaskModel *m = model.getTask(0); + int num_deletes = 0; + QObject::connect(m, &QObject::destroyed, [&num_deletes](){++num_deletes;}); + + model.removeTask(m); + // process deleteLater() events + QCoreApplication::sendPostedEvents(nullptr, QEvent::DeferredDelete); + // as m is owned by model, m should be destroyed + // EXPECT_EQ(num_deletes, 1); // TODO: event is not processed, missing event loop? + EXPECT_EQ(model.rowCount(), 0); +} diff --git a/visualization/motion_planning_tasks/task_list_model.cpp b/visualization/motion_planning_tasks/task_list_model.cpp index 0f685073..eb5828ce 100644 --- a/visualization/motion_planning_tasks/task_list_model.cpp +++ b/visualization/motion_planning_tasks/task_list_model.cpp @@ -143,6 +143,8 @@ public: return q_ptr->createIndex(src.row(), src.column(), src_parent.internalPointer()); } + void removeTask(BaseTaskModel *model); + private: void _q_sourceRowsAboutToBeInserted(const QModelIndex &parent, int start, int end); void _q_sourceRowsInserted(const QModelIndex &parent, int start, int end); @@ -248,12 +250,6 @@ bool TaskListModel::setData(const QModelIndex &index, const QVariant &value, int return task->model_->setData(src_index, value, role); } -bool TaskListModel::removeRows(int row, int count, const QModelIndex &parent) -{ - // TODO - return false; -} - // process a task monitoring message: // update existing RemoteTask, create a new one, // or (if msg.stages is empty) delete an existing one @@ -309,7 +305,8 @@ BaseTaskModel *TaskListModel::getTask(int row) const return d->tasks_.at(row).model_; } -void TaskListModel::insertTask(BaseTaskModel* model, int row) { +void TaskListModel::insertTask(BaseTaskModel* model, int row) +{ Q_D(TaskListModel); if (row < 0 || (size_t)row > d->tasks_.size()) @@ -323,11 +320,6 @@ void TaskListModel::insertTask(BaseTaskModel* model, int row) { d->tasks_.insert(it, TaskListModelPrivate::BaseModelData(model)); endInsertRows(); - // notice destruction of task - if (model->parent() != this) - connect(model, &BaseTaskModel::destroyed, - [this](QObject* o) { removeTask(static_cast(o), false); }); - connect(model, SIGNAL(rowsAboutToBeInserted(QModelIndex,int,int)), this, SLOT(_q_sourceRowsAboutToBeInserted(QModelIndex,int,int))); connect(model, SIGNAL(rowsInserted(QModelIndex,int,int)), @@ -344,40 +336,74 @@ void TaskListModel::insertTask(BaseTaskModel* model, int row) { this, SLOT(_q_sourceDataChanged(QModelIndex,QModelIndex,QVector))); } -bool TaskListModel::removeTask(BaseTaskModel* model, bool disconnect_signals) { +bool TaskListModel::removeTask(BaseTaskModel* model) +{ Q_D(TaskListModel); // find row corresponding to model auto it = std::find_if(d->tasks_.begin(), d->tasks_.end(), - [model](const TaskListModelPrivate::BaseModelData& data) { return data.model_ == model; }); + [model](const TaskListModelPrivate::BaseModelData& data) { + return data.model_ == model; + }); if (it == d->tasks_.end()) return false; // model not found - ROS_DEBUG_NAMED("TaskListModel", "%p: removing task: %p", this, model); - size_t row = it - d->tasks_.begin(); - beginRemoveRows(QModelIndex(), row, row); - d->tasks_.insert(it, TaskListModelPrivate::BaseModelData(model)); - endRemoveRows(); + return removeTasks(it - d->tasks_.begin(), 1); +} - if (disconnect_signals) { // reacting on signal destroyed(), signals are already disconnected - disconnect(model, SIGNAL(rowsAboutToBeInserted(QModelIndex,int,int)), - this, SLOT(_q_sourceRowsAboutToBeInserted(QModelIndex,int,int))); - disconnect(model, SIGNAL(rowsInserted(QModelIndex,int,int)), - this, SLOT(_q_sourceRowsInserted(QModelIndex,int,int))); - disconnect(model, SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int)), - this, SLOT(_q_sourceRowsAboutToBeRemoved(QModelIndex,int,int))); - disconnect(model, SIGNAL(rowsRemoved(QModelIndex,int,int)), - this, SLOT(_q_sourceRowsRemoved(QModelIndex,int,int))); - disconnect(model, SIGNAL(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)), - this, SLOT(_q_sourceRowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int))); - disconnect(model, SIGNAL(rowsMoved(QModelIndex,int,int,QModelIndex,int)), - this, SLOT(_q_sourceRowsMoved(QModelIndex,int,int,QModelIndex,int))); - disconnect(model, SIGNAL(dataChanged(QModelIndex,QModelIndex,QVector)), - this, SLOT(_q_sourceDataChanged(QModelIndex,QModelIndex,QVector))); - } +bool TaskListModel::removeTasks(int row, int count) +{ + Q_D(TaskListModel); + if (row < 0 || row+count > rowCount()) + return false; + + auto first = d->tasks_.begin(); std::advance(first, row); + auto last = first; std::advance(last, count); + beginRemoveRows(QModelIndex(), row, row+count-1); + std::for_each(first, last, [d](TaskListModelPrivate::BaseModelData &data) { + d->removeTask(data.model_); + }); + d->tasks_.erase(first, last); + endRemoveRows(); return true; } +bool TaskListModel::removeRows(int row, int count, const QModelIndex &parent) +{ + Q_D(TaskListModel); + if (!parent.isValid()) { // top-level items = tasks + return removeTasks(row, count); + } else { + TaskListModelPrivate::BaseModelData *data = nullptr; + QModelIndex src_parent = d->mapToSource(parent, &data); + return data->model_->removeRows(row, count, src_parent); + } +} + +void TaskListModelPrivate::removeTask(BaseTaskModel *model) +{ + ROS_DEBUG_NAMED("TaskListModel", "%p: removing task: %p", q_ptr, model); + + QObject::disconnect(model, SIGNAL(rowsAboutToBeInserted(QModelIndex,int,int)), + q_ptr, SLOT(_q_sourceRowsAboutToBeInserted(QModelIndex,int,int))); + QObject::disconnect(model, SIGNAL(rowsInserted(QModelIndex,int,int)), + q_ptr, SLOT(_q_sourceRowsInserted(QModelIndex,int,int))); + QObject::disconnect(model, SIGNAL(rowsAboutToBeRemoved(QModelIndex,int,int)), + q_ptr, SLOT(_q_sourceRowsAboutToBeRemoved(QModelIndex,int,int))); + QObject::disconnect(model, SIGNAL(rowsRemoved(QModelIndex,int,int)), + q_ptr, SLOT(_q_sourceRowsRemoved(QModelIndex,int,int))); + QObject::disconnect(model, SIGNAL(rowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int)), + q_ptr, SLOT(_q_sourceRowsAboutToBeMoved(QModelIndex,int,int,QModelIndex,int))); + QObject::disconnect(model, SIGNAL(rowsMoved(QModelIndex,int,int,QModelIndex,int)), + q_ptr, SLOT(_q_sourceRowsMoved(QModelIndex,int,int,QModelIndex,int))); + QObject::disconnect(model, SIGNAL(dataChanged(QModelIndex,QModelIndex,QVector)), + q_ptr, SLOT(_q_sourceDataChanged(QModelIndex,QModelIndex,QVector))); + + // delete model if we own it + if (model->parent() == q_ptr) + model->deleteLater(); +} + void TaskListModelPrivate::_q_sourceRowsAboutToBeInserted(const QModelIndex &parent, int start, int end) { q_ptr->beginInsertRows(mapFromSource(parent), start, end); diff --git a/visualization/motion_planning_tasks/task_list_model.h b/visualization/motion_planning_tasks/task_list_model.h index 61e7a688..ffd30161 100644 --- a/visualization/motion_planning_tasks/task_list_model.h +++ b/visualization/motion_planning_tasks/task_list_model.h @@ -99,7 +99,8 @@ public: /// insert a TaskModel into our list void insertTask(BaseTaskModel* model, int row = -1); - bool removeTask(BaseTaskModel* model, bool disconnect_signals = true); + bool removeTask(BaseTaskModel* model); + bool removeTasks(int row, int count); private: Q_PRIVATE_SLOT(d_func(), void _q_sourceRowsAboutToBeInserted(QModelIndex,int,int)) diff --git a/visualization/motion_planning_tasks/task_panel.cpp b/visualization/motion_planning_tasks/task_panel.cpp index 52e9639e..fcd37116 100644 --- a/visualization/motion_planning_tasks/task_panel.cpp +++ b/visualization/motion_planning_tasks/task_panel.cpp @@ -81,9 +81,8 @@ TaskPanel::TaskPanel(QWidget* parent) { Q_D(TaskPanel); // connect signals - connect(d->actionNewTask, &QAction::triggered, this, &TaskPanel::onAddTask); - connect(d->actionNewStage, &QAction::triggered, this, &TaskPanel::onAddStage); - connect(d->actionRemoveStages, &QAction::triggered, this, &TaskPanel::onRemoveStages); + connect(d->actionRemoveTaskTreeRows, &QAction::triggered, this, &TaskPanel::removeTaskTreeRows); + connect(d->button_show_stage_dock_widget, &QToolButton::clicked, this, &TaskPanel::showStageDockWidget); } TaskPanel::~TaskPanel() @@ -93,16 +92,16 @@ TaskPanel::~TaskPanel() TaskPanelPrivate::TaskPanelPrivate(TaskPanel *q_ptr) : q_ptr(q_ptr) - , tasks_model(TaskListModelCache::instance().getGlobalModel()) + , task_list_model(TaskListModelCache::instance().getGlobalModel()) , settings(new rviz::PropertyTreeModel(new rviz::Property)) { setupUi(q_ptr); initSettings(settings->getRoot()); settings_view->setModel(settings); - tasks_view->setModel(tasks_model.get()); + tasks_view->setModel(task_list_model.get()); // init actions - tasks_view->addActions({actionNewTask, actionNewStage, actionRemoveStages}); + tasks_view->addActions({actionRemoveTaskTreeRows}); } void TaskPanelPrivate::initSettings(rviz::Property* root) @@ -129,20 +128,20 @@ void TaskPanel::onAddTask() { Q_D(TaskPanel); QModelIndex current = d->tasks_view->currentIndex(); - d_ptr->tasks_model->insertTask(new LocalTaskModel(this), current.row()); + d_ptr->task_list_model->insertTask(new LocalTaskModel(this), current.row()); } -void TaskPanel::onAddStage() +void TaskPanel::showStageDockWidget() { rviz::PanelDockWidget *dock = getStageDockWidget(vis_manager_->getWindowManager()); dock->show(); } -void TaskPanel::onRemoveStages() +void TaskPanel::removeTaskTreeRows() { Q_D(TaskPanel); for (const auto &range : d_ptr->tasks_view->selectionModel()->selection()) - d_ptr->tasks_model->removeRows(range.top(), range.bottom()-range.top()+1, range.parent()); + d_ptr->task_list_model->removeRows(range.top(), range.bottom()-range.top()+1, range.parent()); } } diff --git a/visualization/motion_planning_tasks/task_panel.h b/visualization/motion_planning_tasks/task_panel.h index b849d1f5..5eb5a830 100644 --- a/visualization/motion_planning_tasks/task_panel.h +++ b/visualization/motion_planning_tasks/task_panel.h @@ -70,8 +70,8 @@ public: public Q_SLOTS: void onAddTask(); - void onAddStage(); - void onRemoveStages(); + void showStageDockWidget(); + void removeTaskTreeRows(); }; } diff --git a/visualization/motion_planning_tasks/task_panel.ui b/visualization/motion_planning_tasks/task_panel.ui index 98dd8e09..efaadcf4 100644 --- a/visualization/motion_planning_tasks/task_panel.ui +++ b/visualization/motion_planning_tasks/task_panel.ui @@ -30,7 +30,7 @@ - + 18 @@ -66,36 +66,18 @@ - - - - .. - + - New Stage + Remove - Add a new stage + Remove selected rows - - - - - .. + + Del - - New Task - - - Add a new task - - - - - Remove Stages - - - Remove selected stages + + Qt::WidgetShortcut @@ -107,22 +89,5 @@ - - - button_add_stage - clicked() - actionNewStage - trigger() - - - 378 - 21 - - - -1 - -1 - - - - + diff --git a/visualization/motion_planning_tasks/task_panel_p.h b/visualization/motion_planning_tasks/task_panel_p.h index 5555c4be..f7bbd6dc 100644 --- a/visualization/motion_planning_tasks/task_panel_p.h +++ b/visualization/motion_planning_tasks/task_panel_p.h @@ -53,7 +53,7 @@ public: void initSettings(rviz::Property *root); TaskPanel* q_ptr; - TaskListModelPtr tasks_model; + TaskListModelPtr task_list_model; rviz::PropertyTreeModel* settings; };