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

Implement low-level interface moveit_cpp #1656

Merged
merged 3 commits into from
Nov 21, 2019

Conversation

henningkayser
Copy link
Member

As already discussed in the last maintainer meeting we at PickNik have a need for a set of low-level interface classes that allow us to use, modify and exchange core components without the overhead
to adjust message types. Also, we are implementing performance-critical functions that are not throttled by the ROS ecosystem. The first MVP is a MoveGroup-like set of features that allows basic planning and execution for different planning groups. I will follow-up with more detailed explanation of the goals, requirements, roadmap and design decisions.

@henningkayser henningkayser changed the title WIP: Implement low-level interface moveit_cpp Implement low-level interface moveit_cpp Sep 25, 2019
@henningkayser henningkayser removed the wip label Sep 25, 2019
Copy link
Member

@davetcoleman davetcoleman left a comment

Choose a reason for hiding this comment

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

Some minor documentation / cleanup

@@ -0,0 +1,12 @@
set(MOVEIT_LIB_NAME moveit_cpp)

add_library(${MOVEIT_LIB_NAME} src/moveit_cpp.cpp src/planning_component.cpp)
Copy link
Member

Choose a reason for hiding this comment

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

list on individual lines


add_library(${MOVEIT_LIB_NAME} src/moveit_cpp.cpp src/planning_component.cpp)
set_target_properties(${MOVEIT_LIB_NAME} PROPERTIES VERSION "${${PROJECT_NAME}_VERSION}")
target_link_libraries(${MOVEIT_LIB_NAME} moveit_common_planning_interface_objects moveit_planning_scene_interface ${catkin_LIBRARIES} ${Boost_LIBRARIES})
Copy link
Member

Choose a reason for hiding this comment

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

individual lines

* POSSIBILITY OF SUCH DAMAGE.
*********************************************************************/

/* Author: Henning Kayser */
Copy link
Member

Choose a reason for hiding this comment

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

Add description of what this file does

std::string publish_planning_scene_topic;
double wait_for_initial_state_timeout;
};
struct PlanningPipelineOptions
Copy link
Member

Choose a reason for hiding this comment

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

Add desc comment

std::vector<std::string> pipeline_names;
std::string parent_namespace;
};
struct PlannerOptions
Copy link
Member

Choose a reason for hiding this comment

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

Add desc comment

}
// TODO(henningkayser): fix and remove lines below
ros::spinOnce();
ros::Duration(0.5).sleep(); // when at 0.1, i believe sometimes vjoint not properly loaded
Copy link
Member

Choose a reason for hiding this comment

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

is this comment old from my boilerplate code? it looks familar to me. i think we should remove the sleep.

Copy link
Member

Choose a reason for hiding this comment

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

ping: remove the sleep


robot_model::RobotModelConstPtr MoveItCpp::getRobotModel() const
{
ROS_DEBUG_NAMED(LOGNAME, "MoveItCpp::getRobotModel()");
Copy link
Member

Choose a reason for hiding this comment

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

remove these loggers?


const ros::NodeHandle& MoveItCpp::getNodeHandle() const
{
ROS_DEBUG_NAMED(LOGNAME, "MoveItCpp::getNodeHandle()");
Copy link
Member

Choose a reason for hiding this comment

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

remove these loggers.. this is a simple getter

const planning_scene_monitor::PlanningSceneMonitorPtr& MoveItCpp::getPlanningSceneMonitor() const
{
return planning_scene_monitor_;
}
Copy link
Member

Choose a reason for hiding this comment

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

add line break

*********************************************************************/

/* Author: Jafar Abdi
Desc: Test the MoveItCpp and PlanningComponent interfaces
Copy link
Member

Choose a reason for hiding this comment

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

Love that there's a Desc here!

Copy link
Member

@davetcoleman davetcoleman left a comment

Choose a reason for hiding this comment

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

This is looking really good! Why is Travis failing still?

}
// TODO(henningkayser): fix and remove lines below
ros::spinOnce();
ros::Duration(0.5).sleep(); // when at 0.1, i believe sometimes vjoint not properly loaded
Copy link
Member

Choose a reason for hiding this comment

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

ping: remove the sleep

@davetcoleman
Copy link
Member

/root/ros_ws/src/moveit/moveit_ros/planning_interface/moveit_cpp/include/moveit/moveit_cpp/planning_component.h:191:21: warning: ‘moveit::planning_interface::PlanningComponent::group_name_’ will be initialized after [-Wreorder] const std::string group_name_;

@JafarAbdi
Copy link
Contributor

The last commit fix the kinetic error, please see

@henningkayser
Copy link
Member Author

henningkayser commented Nov 21, 2019

Cleaned up commit history and rebased onto master, lgtm

@henningkayser henningkayser merged commit f0321f0 into moveit:master Nov 21, 2019
@davetcoleman
Copy link
Member

Hurray! v0 of moveit_cpp has been merged. @henningkayser can you announce this on MoveIt Discourse?

@rhaschke
Copy link
Contributor

rhaschke commented Nov 22, 2019

@JafarAbdi
Copy link
Contributor

@rhaschke Thank you for reporting! I'm working on it right now!

I just tried to run the test forever and indeed sometimes I get seg fault

ros.moveit_ros_planning_interface.moveit_cpp: MoveItCpp running
Segmentation fault (core dumped)

@henningkayser
Copy link
Member Author

@rhaschke @v4hn We've debugged this and @JafarAbdi will file a PR for this. The issue is caused by the fake controller not publishing valid joint states for joints not configured in the initial state. In this particular case panda_moveit_config had an initial state ready for the full group which didn't include finger joints, leading to undefined values being published. The PR will fix this by publishing 0 values by default.

@JafarAbdi JafarAbdi mentioned this pull request Nov 26, 2019
6 tasks
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

4 participants