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

Port moveit ros visualization #160

Merged
merged 7 commits into from
Jan 31, 2020

Conversation

JafarAbdi
Copy link
Member

@JafarAbdi JafarAbdi commented Jan 22, 2020

Description

Depends on common_planning_interface_objects and planning_scene_monitor

rviz_plugin_render_tools and robot_state_rviz_plugin are ported the remaining one for the beta release is planning_scene_rviz_plugin (should we port the remaining two .?)

TODO:

  • Port package.xml

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@JafarAbdi
Copy link
Member Author

@nbbrooks

@JafarAbdi JafarAbdi force-pushed the pr-moveit_ros_visualization branch 4 times, most recently from 3140603 to 49b6049 Compare January 28, 2020 15:10
@JafarAbdi JafarAbdi changed the title WIP: Port moveit ros visualization Port moveit ros visualization Jan 28, 2020
Copy link
Member

@henningkayser henningkayser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, these are a lot of changes! Looks good to me for the most part. Did you get to load/run this with RViz?

moveit_core/utils/include/moveit/utils/rclcpp_utils.h Outdated Show resolved Hide resolved
@@ -575,7 +580,9 @@ void PlanningSceneDisplay::onEnable()
{
Display::onEnable();

addBackgroundJob(boost::bind(&PlanningSceneDisplay::loadRobotModel, this), "loadRobotModel");
addBackgroundJob(
boost::bind(&PlanningSceneDisplay::loadRobotModel, this, rclcpp::Node::make_shared("planning_scene_display")),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we just pass the node or create a subnode?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure which one is better, what do you think .?

@mlautman
Copy link
Contributor

Building this, I am able to see the RobotState Display but not the Planning scene. When I try to load the RobotState Display, I get the following error. Is that expected?

[ERROR] [rviz2]: PluginlibFactory: The plugin for class 'moveit_rviz_plugin/RobotState' failed to load. Error: Failed to load library /home/mike/ws_moveit2/install/moveit_ros_visualization/lib/libmoveit_robot_state_rviz_plugin.so. Make sure that you are calling the PLUGINLIB_EXPORT_CLASS macro in the library code, and that names are consistent between this macro and your XML. Error string: Could not load library (Poco exception = /home/mike/ws_moveit2/install/moveit_ros_visualization/lib/libmoveit_rviz_plugin_render_tools.so.: undefined symbol: _ZTVN18moveit_rviz_plugin15TrajectoryPanelE)

Regarding your question about the other two displays: Trajectory would be useful to have if it isn't too much work. The Motion planning one would be a huge win but it depends on a ton of other pieces so it would likely need to wait until after at least move_group.

@mlautman
Copy link
Contributor

I believe you can also remove the <rviz plugin="..."/> tags from the package.xml

Copy link
Member

@henningkayser henningkayser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the remaining points and squash the commit history a bit

@henningkayser henningkayser merged commit eb44725 into moveit:master Jan 31, 2020
@RoboticsYY RoboticsYY mentioned this pull request May 26, 2020
1 task
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.

None yet

3 participants