ros / meta-ros

OpenEmbedded Layers for ROS 1 and ROS 2
MIT License
391 stars 255 forks source link

{humble}: fix host path is included in cmake files. #1192

Closed jiaxshi closed 3 weeks ago

jiaxshi commented 5 months ago

As mentioned in #1166 HOST path is included in cmake files. This is because urdfdom dependes on tinyxml which is build via makefile and it doesn't provide tinyxml.cmake. And urdfdom provides a FindTinyXML.cmake to find it. Then absolute path is used in FindTinyXML.cmake.

There is an issue raised in urdfdom projet 202. And urdfdom group had discussed this issue in https://discourse.ros.org/t/upcoming-api-break-in-urdfdom/34750. And they decided to use tinyxml2 which has good maintenance(tinyxml is no longer maintained). This is merged in master branch(https://github.com/ros/urdfdom/pull/186).

Since with latest version of urdfdom, the issue should not present with tinyxml2 is used. So the PR is a workaround to remove host path in cmakes.

robwoolley commented 4 months ago

Thanks for providing this fix. I haven't gotten to merging the ros_opt_prefix work into kirkstone yet, but I should be getting to that very soon.

What steps do you take to reproduce the problem? Is there a particular recipe that depends on urdfdom that demonstrates the problem?

jiaxshi commented 4 months ago

Hi Rob, No special case. It's an unexpected discovery while using check_urdf and urdf_to_graphviz commands.

sh-5.1# check_urdf 1.urdf
robot name is: myfirst
---------- Successfully Parsed XML ---------------
root Link: base_link has 0 child(ren)
sh-5.1# urdf_to_graphviz ../1.urdf
WARNING: OUTPUT not given. This type of usage is deprecated!Usage: urdf_to_graphviz input.xml [OUTPUT]  Will create either $ROBOT_NAME.gv & $ROBOT_NAME.pdf in CWD  or OUTPUT.gv & OUTPUT.pdf.
Created file myfirst.gv
Created file myfirst.pdf
sh-5.1#
robwoolley commented 3 weeks ago

@jiaxshi Thanks for reporting this. Based on the info you provided, I decided to take the approach of backporting the fix you pointed me to. I think this has additional benefits including eliminating a dependency that is no longer maintained: https://github.com/ros/meta-ros/commit/8415d7c8d4f6d28a9624324a517e16ea031bd5bd

I have also opened an issue so that I may port it to other combos: https://github.com/ros/meta-ros/issues/1250