-
Notifications
You must be signed in to change notification settings - Fork 523
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
Latched Strings for URDF and SRDF #765
Conversation
I tested with this branch of |
Codecov Report
@@ Coverage Diff @@
## main #765 +/- ##
==========================================
+ Coverage 60.79% 60.85% +0.06%
==========================================
Files 271 273 +2
Lines 23823 23880 +57
==========================================
+ Hits 14482 14529 +47
- Misses 9341 9351 +10
Continue to review full report at Codecov.
|
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.
Implementation looks good to me so far. We should get a version of this feature merged to unblock MSA work, I would only like to at least make sure the API is compatible with switching to using synced parameters in the future. With this implementation, I don't see too many issues with this, though.
SRDF and URDF need to be checked for compatibility, otherwise the callbacks are risky to use. Possibly switch to using a single callback that triggers when both values are present.
Do the topics use a global namespace?
moveit_ros/planning/rdf_loader/include/moveit/rdf_loader/rdf_loader.h
Outdated
Show resolved
Hide resolved
moveit_ros/planning/rdf_loader/include/moveit/rdf_loader/string_loader.h
Outdated
Show resolved
Hide resolved
One thing I'm pondering is how to specify parameters while maintaining maximum compatibility. The goal was to keep the
But if we want to have |
This pull request is in conflict. Could you fix it @DLu? |
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.
I generally really like this. I just have a small question about the base QoS you are using.
Added documentation in moveit/moveit2_tutorials#234 |
Description
Initial solution for #755 / #169
rdf_loader::StringLoader
class which loads a string from the ROS environment, either via parameter or topic (could theoretically be extended for dynamic parameter later)robot_description
androbot_description_semantic
std_msgs::msg::String
if read in from a parameter.RDFLoader
can take appropriate actions when the description is updated.Checklist