-
Notifications
You must be signed in to change notification settings - Fork 515
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
Port moveit ros visualization #160
Conversation
3140603
to
49b6049
Compare
moveit_ros/visualization/robot_state_rviz_plugin/src/robot_state_display.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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?
...lanning_scene_rviz_plugin/include/moveit/planning_scene_rviz_plugin/planning_scene_display.h
Show resolved
Hide resolved
...lanning_scene_rviz_plugin/include/moveit/planning_scene_rviz_plugin/planning_scene_display.h
Outdated
Show resolved
Hide resolved
moveit_ros/visualization/planning_scene_rviz_plugin/src/planning_scene_display.cpp
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")), |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 .?
moveit_ros/visualization/robot_state_rviz_plugin/src/robot_state_display.cpp
Outdated
Show resolved
Hide resolved
...ion/rviz_plugin_render_tools/include/moveit/rviz_plugin_render_tools/planning_link_updater.h
Outdated
Show resolved
Hide resolved
moveit_ros/visualization/rviz_plugin_render_tools/include/ogre_helpers/mesh_shape.hpp
Show resolved
Hide resolved
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?
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. |
I believe you can also remove the |
...lanning_scene_rviz_plugin/include/moveit/planning_scene_rviz_plugin/planning_scene_display.h
Outdated
Show resolved
Hide resolved
moveit_ros/visualization/planning_scene_rviz_plugin/src/planning_scene_display.cpp
Outdated
Show resolved
Hide resolved
moveit_ros/visualization/planning_scene_rviz_plugin/src/planning_scene_display.cpp
Outdated
Show resolved
Hide resolved
ea2e35c
to
6371c7b
Compare
There was a problem hiding this 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
moveit_ros/visualization/rviz_plugin_render_tools/include/ogre_helpers/mesh_shape.hpp
Show resolved
Hide resolved
e10561b
to
c05f606
Compare
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:
Checklist