Fix xacro args loading issue

[alexnavtt]

In certain cases, the xacro args provided to moveit_setup_assistant were not loaded properly by moveit_configs_builder. This was due to an incorrect indentation in moveit_configs_builder.py which had the xacro args loaded only if self.__urdf_package was None which seemed incorrect to me. Now it depends only on the existence of the urdf config file.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 42.38%. Comparing base (d962501) to head (62b7211).
Report is 19 commits behind head on main.

❗ Current head 62b7211 differs from pull request most recent head fb1583e. Consider uploading reports for the commit fb1583e to get more accurate results

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.
📢 Have feedback on the report? Share it here.

[tylerjw]

Please run pre-commit to format your python code with black.

[alexnavtt]

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?

[sjahr]

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

[alexnavtt]

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]

Thank you for linking the code and the explanation. Do you mind opening a separate PR to add this to the humble branch?

[alexnavtt]

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?

[sjahr]

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

[alexnavtt]

Still have checks failing which I don't believe are on my end