Restructure code to remove the need for multiple maps.

[Levi-Armstrong]

It was becoming very tedious to manage all of the local mapping so I restructure the code to remove all of the map simplifying the code and should make it easier to manage. This is to address issue #93 and includes pull request #87.

[Levi-Armstrong]

@gavanderhoorn It was becoming a pain to manage all of the local mapping variable so I restructured the code to remove them. Take a look and let me know what you think.

[gavanderhoorn]

@Levi-Armstrong: thanks, this is definitely an improvement. It looks like you:

• introduce sub classes of QTreeWidgetItem: one for items representing links and one for joints
• extend the URDFPropertyTree class introduced by @swhart115

Then the responsibility of keeping the state of the tree widget in sync with the in-memory urdf model (ie: on change, add, delete, etc) is largely transferred to the URDFPropertyTree class, and the link and joint tree item instances basically implement a 1-element 'map' between links and joints and their corresponding tree items.

This all on-top of the changes introduced by @swhart115 to support drag-and-drop of links.

Is that a correct summary of the changes in this PR?

[gavanderhoorn]

Could you please change this to use angle brackets (< and >)?

[Levi-Armstrong]

@gavanderhoorn : Your summary is correct.

@Levi-Armstrong: thanks, this is definitely an improvement. It looks like you:

introduce sub classes of QTreeWidgetItem: one for items representing links and one for joints
extend the URDFPropertyTree class introduced by @swhart115
Then the responsibility of keeping the state of the tree widget in sync with the in-memory urdf model (ie: on change, add, delete, etc) is largely transferred to the URDFPropertyTree class, and the link and joint tree item instances basically implement a 1-element 'map' between links and joints and their corresponding tree items.

This all on-top of the changes introduced by @swhart115 to support drag-and-drop of links.

[gavanderhoorn]

I've just pushed the Milestone 1 merge candidate that includes PRs #70, #88 and #95, see m1_merge_candidate (diff here).

As this PR contains quite few changes, I haven't yet merged it into m1_merge_candidate, but it will end up there. I'll see if I can rebase your changes on-top of m1_merge_candidate and push it to the repository. I might need some help though as a lot of things have been moved around that have also been touched by the other PRs.

[gavanderhoorn]

@Levi-Armstrong: merging was easier than rebasing, but even that is proving a challenge. The refactor to urdf_property.(h|cpp) make it hard to understand what should be / has moved to urdf_property_tree.(h|cpp). The other conflicts are relatively easily resolved. Could you give it a try?

[Levi-Armstrong]

@Levi-Armstrong: merging was easier than rebasing, but even that is proving a challenge. The refactor to urdf_property.(h|cpp) make it hard to understand what should be / has moved to urdf_property_tree.(h|cpp). The other conflicts are relatively easily resolved. Could you give it a try?

Yea, I will look at it now.

[Levi-Armstrong]

@gavanderhoorn, I believe everything has been merged correctly. The only item I was not certain on was the TF portion. To have everything work smoothly, I flowed all of the signals from the JointProperty to the URDFPropertyTree. This allowed the URDFProperty to monitor changes at a more detailed level allowing for the TF to only reside in the URDFProperty. Also this prevents the model from reloading when not necessary.

[gavanderhoorn]

I've merged this into m1_merge_candidate_v2 (diff).

Thanks for taking the time to merge m1_merge_candidate for me. That helped. 💯

re: tf display + class: yes, that seems to not work 100% now: TF frames don't show up until one of the associated properties changes (ie: origin, link names, etc). If I understand things correctly, you removed the call to URDFTransformer::updateLink(JointProperty*) (here), which was responsible for getting new joints into the TF tree. I followed all the new signals you introduced (:+1:) but couldn't quite make out the flow to fix this. Basically, URDFTransformer needs to be notified of newly added joints. Could be that hooking linkAddition(..) and jointAddition(..) would be sufficient for this.

[Levi-Armstrong]

@gavanderhoorn, I have fixed the issues so TF will show up when a new model is loaded. Take a look and let me know what you think.

[gavanderhoorn]

If you want to test the m1_merge_candidate_v2 (which does not yet include your latest commit): I installed the OCE packages from the ppa:freecad-maintainers/oce-release PPA.

[Levi-Armstrong]

@gavanderhoorn; I tried testing the the new branch but it crashes immediately. It looks like using pcl with c++11 may be the issue.

[gavanderhoorn]

Yes, I should've updated the installation/setup instructions: that branch needs to be build in release mode, otherwise it'll crash. It's indeed related to pcl and c++11. See #96.

so for catkin_make: catkin_make -DCMAKE_BUILD_TYPE=RelWithDebInfo. For catkin_tools you can add that to your profile.

I might add a check in the CMakeLists.txt that sets the build type to release if it isn't set by the user.

[gavanderhoorn]

Running just this branch gets me a SEGFAULT as soon as I try to remove a link. Have you ran into that?

[gavanderhoorn]

Stack trace:

Program received signal SIGSEGV, Segmentation fault.
QTreeWidgetItem::addChild (this=this@entry=0x22555e0, child=0x1a76f30)
    at itemviews/qtreewidget.cpp:1886
1886    itemviews/qtreewidget.cpp: No such file or directory.
(gdb) bt
#0  QTreeWidgetItem::addChild (this=this@entry=0x22555e0, child=0x1a76f30)
    at itemviews/qtreewidget.cpp:1886
#1  0x0000000000453b78 in urdf_editor::URDFPropertyTree::moveTreeChildren (
    this=this@entry=0xb02680, parent=parent@entry=0x22554d0, 
    new_parent=new_parent@entry=0x22555e0)
    at /home/user/cad2ros/src/cad-to-ros/urdf_editor/src/urdf_property_tree.cpp:414
#2  0x0000000000453fc8 in urdf_editor::URDFPropertyTree::removeLinkTreeItem (
    this=this@entry=0xb02680, item=item@entry=0x22554d0)
    at /home/user/cad2ros/src/cad-to-ros/urdf_editor/src/urdf_property_tree.cpp:306
#3  0x0000000000454741 in urdf_editor::URDFPropertyTree::on_removeActionTriggered (this=0xb02680)
    at /home/user/cad2ros/src/cad-to-ros/urdf_editor/src/urdf_property_tree.cpp:472
#4  0x000000000042dc25 in urdf_editor::URDFPropertyTree::qt_static_metacall (
    _o=0x22555e0, _c=27750192, _id=27750192, _a=0x21a7a90)
    at /home/user/cad2ros/build/urdf_builder/include/urdf_editor/moc_urdf_property_tree.cxx:116
#5  0x00007ffff502b2da in QMetaObject::activate (sender=sender@entry=0xb2c570, 
    m=m@entry=0x7ffff5ff0de0 <QAction::staticMetaObject>, 
    local_signal_index=local_signal_index@entry=1, 
    argv=argv@entry=0x7fffffffb8c0) at kernel/qobject.cpp:3539
---Type <return> to continue, or q <return> to quit---q
Quit
(gdb) frame 1
#1  0x0000000000453b78 in urdf_editor::URDFPropertyTree::moveTreeChildren (
    this=this@entry=0xb02680, parent=parent@entry=0x22554d0, 
    new_parent=new_parent@entry=0x22555e0)
    at /home/user/cad2ros/src/cad-to-ros/urdf_editor/src/urdf_property_tree.cpp:414
414           new_parent->addChild(parent->takeChild(i));

[Levi-Armstrong]

@gavanderhoorn; I do not get that error when working of this branch but I do when using the _V2 branch. If you do a clean build does the error still occur?

[Levi-Armstrong]

@gavanderhoorn; The issue occurs when compiling with c++11. I took the m1_merged_candidate_v2 and removed all of the c++11 items and everything works as expected.

[gavanderhoorn]

@Levi-Armstrong: I've cherry-picked 70b2b44 into the merge candidate v2 branch.

[gavanderhoorn]

All commits from this PR have been merged into m1_merge_candidate_v2, so I'll close this PR.

Any additional issues with the merged functionality will be fixed in later issues/PRs.

[gavanderhoorn]

Thanks @Levi-Armstrong 💯.