-
Notifications
You must be signed in to change notification settings - Fork 581
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
Parse xacro args from .setup_assistant config in MoveIt Configs Builder #2172
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #2172 +/- ##
==========================================
- Coverage 50.49% 50.47% -0.02%
==========================================
Files 387 387
Lines 31790 31790
==========================================
- Hits 16050 16043 -7
- Misses 15740 15747 +7 ☔ View full report in Codecov by Sentry. |
moveit_configs_utils/moveit_configs_utils/moveit_configs_builder.py
Outdated
Show resolved
Hide resolved
Servo tests are flaky, I just retriggered CI |
Gotcha, thanks! |
if (mappings is None) or all( | ||
(isinstance(key, str) and isinstance(value, str)) | ||
for key, value in mappings.items() | ||
): | ||
try: | ||
self.__moveit_configs.robot_description = { | ||
self.__robot_description: load_xacro( | ||
robot_description_file_path, mappings=mappings | ||
robot_description_file_path, | ||
mappings=mappings or self.__urdf_xacro_args, |
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.
Why do we need this or
when you assigned mappings
on L226 (does 226 need to be removed like Jafar suggested)?
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.
Yeah, I think that line needs to be removed
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.
The assignment should be removed now.
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 thought I cut it out of there but I must have accidentally undo'd that change. Thank you @JafarAbdi for cleaning that up for me
moveit_configs_utils/moveit_configs_utils/moveit_configs_builder.py
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.
I was not able to test these changes but they look good to me.
* Parse xacro args from .setup_assistant config in MoveIt Configs Builder (#2172) Co-authored-by: Jafar <jafar.uruc@gmail.com> (cherry picked from commit 79a2fa5) * Fix xacro args loading issue (#2684) * Fixed xacro args loading issue * Formatting fixes with pre-commit action --------- (cherry picked from commit cdb20ae) --------- Co-authored-by: Anthony Baker <abake48@users.noreply.github.com>
Description
Update MoveIt Config Builder to parse xacro_args parameter from MSA generated MoveIt 2 configs that use xacro generated robot descriptions.
Checklist