Skip to content

Port rdf_loader to ROS 2#76

Closed
vmayoral wants to merge 3 commits intomoveit:masterfrom
AcutronicRobotics:rdf-loader2
Closed

Port rdf_loader to ROS 2#76
vmayoral wants to merge 3 commits intomoveit:masterfrom
AcutronicRobotics:rdf-loader2

Conversation

@vmayoral
Copy link
Copy Markdown
Contributor

@vmayoral vmayoral commented Apr 5, 2019

No description provided.

@vmayoral vmayoral mentioned this pull request Apr 5, 2019
@vmayoral
Copy link
Copy Markdown
Contributor Author

vmayoral commented Apr 8, 2019

Same as #62 (comment). Please consider these changes.

ros::NodeHandle nh("~");
auto start = std::chrono::system_clock::now();
// TODO (anasarrak): Add the correct node_handle name
auto node = rclcpp::Node::make_shared("/");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a big TODO. What is our strategy for Nodes in MoveIt2? Should we always check that rclcpp has been initialized first? Seems like something we should do with intention.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: change the api for rdf_loader so that the user must pass in a node.


std::string cmd = "rosrun xacro xacro ";
//TODO (anasarrak): Test ros2 xacro https://github.com/ros/xacro/tree/ros2 // https://github.com/bponsler/xacro
std::string cmd = "ros2 run xacro xacro";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we have a demonstration of the RDF Loader loading it loading a robot description? Seems like a test that we should really consider adding

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've actually used xacro to transform from a .urdf.xacro to a .urdf for our models. The problem is that there is actually a ros2 branch for xacro, but is not oficially ported to ros2, the cmakelist is still with catkin... There is an open PR but I've been having problems with it.

The only working one is this one, and I managed to transform the urdf.xacro with the following command:

xacro --inorder mara_robot_camera_top.urdf.xacro -o mara_robot_camera_top.urdf

#include <algorithm>
#include <chrono>

rclcpp::Logger LOGGER = rclcpp::get_logger("moveit").get_child("rdf_loader");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this should be inside a namespace like the other loggers.
If you wanted to use namespace rdf_loader then you could also find replace rdf_loader::RDFLoader:: with RDFLoader:: for every method in the namespace block

@henningkayser
Copy link
Copy Markdown
Member

Closed in favor of #104

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.

5 participants