Fix xacro args loading issue#2684
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2684 +/- ##
==========================================
- Coverage 50.74% 42.38% -8.36%
==========================================
Files 392 692 +300
Lines 32553 56327 +23774
Branches 0 7272 +7272
==========================================
+ Hits 16517 23867 +7350
- Misses 16036 32296 +16260
- Partials 0 164 +164 ☔ View full report in Codecov by Sentry. |
|
Please run pre-commit to format your python code with black. |
|
Also, as an aside, xacro arg loading was introduced for Iron, but not for Humble. Is there a reason for that or did it simply fall through the cracks? |
Maybe that was not properly backported or it is a breaking change in which case we did not backport it to humble. Can you link the corresponding PR? Our backporting strategy is document in this blogpost: https://moveit.ros.org/moveit%202/ros/2023/05/31/balancing-stability-and-development.html |
|
Yeah of course. Here's the initial PR into main (#2172). Can't find a separate commit for adding to the Iron branch so must have been a rebase. With that version of the code, my robot always loaded without attachments that used xacro args because I was providing a urdf package so that branch of the code was never taken. Seems to me more like a bugfix than anything since without it the config builder doesn't work properly. |
sjahr
left a comment
There was a problem hiding this comment.
Thank you for linking the code and the explanation. Do you mind opening a separate PR to add this to the humble branch?
|
Sure thing. I'll wait until this PR is merged then I'll cherry pick the two commits to get it in. However, one of the CI tests seems to be failing and I don't know how to fix that. Any suggestions? |
Thanks, this test failure is not caused by these changes but the test is sometimes flacky. I'll merge main into this PR one last time and then we can merge it |
|
Still have checks failing which I don't believe are on my end |
* Fixed xacro args loading issue * Formatting fixes with pre-commit action --------- Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> (cherry picked from commit cdb20ae)
* 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>
In certain cases, the xacro args provided to
moveit_setup_assistantwere not loaded properly bymoveit_configs_builder. This was due to an incorrect indentation inmoveit_configs_builder.pywhich had the xacro args loaded only ifself.__urdf_packagewasNonewhich seemed incorrect to me. Now it depends only on the existence of the urdf config file.