Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tesseract rviz v3 #33

Merged
merged 67 commits into from
Feb 27, 2023
Merged

Tesseract rviz v3 #33

merged 67 commits into from
Feb 27, 2023

Conversation

marrts
Copy link
Contributor

@marrts marrts commented Jul 27, 2022

Yet another PR trying to port all the latest tesseract_rviz, Built on top of current PR #30. The ultimate goal is to have this basically in line with the latest tesseract_ros package. Currently things are pretty messy, but wanted to get this up there so people are aware it is going on.

Guilherme and others added 30 commits July 28, 2022 16:36
… SimpleResourceLocator by tesseract_common::TesseractSupportResourceLocator
This ports the TesseractState and TesseractTrajectory Widgets to work in ROS2
Clean up cmake
@marrts
Copy link
Contributor Author

marrts commented Aug 5, 2022

Currently waiting on the debian for ros-foxy-qt-advanced-docking so that CI can pass. At this point the ROS2 functionality should nearly be identical to the ROS functionality. The monitor is ported, this was done by just spinning off a separate multithreaded executor node with reentrant callbacks as proposed by @jdlangs as a simple fix even if not the optimal way of doing things. Also, the workbench is essentially fully functional, with some minor differences that I note below. I used the method of spinning off a new node in some of the widgets if needed since the RVIZ node is only single threaded. It's definitely possible that I could've done something improperly or poorly in trying to do this, so I am open to feedback or suggested changes.

Can @Levi-Armstrong or someone else pull this down and test the car seat example to make sure everything is working on not just my machine? Note, I had to modify the car seat example in tesseract_examples to not include the plotter->waitForInput() because using ROS2 launch won't register your inputs to the terminal. At this point I think the code is in a good place to merge as it is functional, but there are still some issues that should be addressed that I can make individual issues for and we can address over time.

Issues:

  • Topic fields in workbench rviz are just string fields because it would crash RVIZ when trying to use them as rostopic fields
  • Can't run waitForInput() when running node from a launch file (as mentioned above)
  • Manipulation widget doesn't currently allow for actually clicking on the interactive markers, but sliders and numerical inputs work
  • There is some building issue that causes rebuilds to fail when a dependency modified, it seems to be related to how rviz_common is exporting it's dependencies, current work around is just delete the install and build directories of tesseract_rviz whenever trying to rebuild it. If somebody can find a way to fix this in the tesseract_rviz CMakeLists.txt though that would be awesome
  • If you want to load the URDF/SRDF into RVIZ as parameters you need to launch RVIZ node with those parameters declared
  • Currently tesseract_rviz uses boost::shared_ptrs everywhere because that's what ROS1 rviz did, but ROS2 uses std::shared_ptrs so we should probably update to match
  • The planning server itself isn't yet ported

@Levi-Armstrong
Copy link
Contributor

Need to install the following qtbase5-private-dev

@Levi-Armstrong
Copy link
Contributor

I added it to the package.xml and restarted CI

@jdlangs jdlangs mentioned this pull request Aug 12, 2022
@Levi-Armstrong
Copy link
Contributor

@marrts Any reason to not merge this in at this point?

@marrts
Copy link
Contributor Author

marrts commented Nov 3, 2022

Still has some of the issues listed above, but at this point it's working reliably. I'm happy with merging it and further bug fixes can be made as they are found and fixed.

@Levi-Armstrong
Copy link
Contributor

Lets do that. Do you mind creating either individual or a single issue which capture the remaining things that need to be addressed?

@marrts marrts mentioned this pull request Nov 9, 2022
7 tasks
@marrts
Copy link
Contributor Author

marrts commented Nov 9, 2022

Made #34 to account for the issues observed

@marrts
Copy link
Contributor Author

marrts commented Dec 9, 2022

Is there anything holding this up from merging at this point?

@Levi-Armstrong
Copy link
Contributor

I would get CI passing then merge.

@marrts marrts merged commit d8e8d8b into tesseract-robotics:ros2-dev Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants