Skip to content

Port rdf_loader to ROS2 #104

Merged
henningkayser merged 5 commits intomoveit:masterfrom
JafarAbdi:pr-rdf_loader
Dec 6, 2019
Merged

Port rdf_loader to ROS2 #104
henningkayser merged 5 commits intomoveit:masterfrom
JafarAbdi:pr-rdf_loader

Conversation

@JafarAbdi
Copy link
Copy Markdown
Member

Description

This is a continuation of PR#76

The addressed review was added to the master branch see but because I'm not able to cherry-pick the changes for the specific files of rdf_loader, I just applied the changes to the rdf_loader and committed them

Copy link
Copy Markdown
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.

This looks pretty good so far. +1 from me after you fixed up the remaining changes

This was referenced Nov 21, 2019
@JafarAbdi JafarAbdi force-pushed the pr-rdf_loader branch 3 times, most recently from 0739631 to 83c9ba4 Compare November 30, 2019 11:59
std::string content;
auto start = std::chrono::system_clock::now();

if (!nh.searchParam(robot_description, robot_description_) || !nh.getParam(robot_description_, content))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure I'm happy with the new implementation, has_parameter() + get_parameter seems like an overhead to me. Also, we need to discuss how searchParam() should be implemented. @mkhansen-intel do you have any suggestions for this part?

@henningkayser henningkayser merged commit 7c77c08 into moveit:master Dec 6, 2019
MikeWrock pushed a commit to MikeWrock/moveit2 that referenced this pull request Aug 15, 2022
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.

3 participants