Skip to content
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

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

Closed
wants to merge 2 commits into from

Conversation

jiaxshi
Copy link
Contributor

@jiaxshi jiaxshi commented Jun 28, 2024

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.

jiaxshi added 2 commits June 28, 2024 11:20
urdfdom-xxx.cmake will used by other packages in cross compilation.
If HOST path is removed, they cannot find dependencies properly. Use
CMAKE_SYSROOT instead.

Signed-off-by: Jiaxing Shi <[email protected]>
@robwoolley
Copy link
Collaborator

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
Copy link
Contributor Author

jiaxshi commented Jul 15, 2024

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
Copy link
Collaborator

@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: 8415d7c

I have also opened an issue so that I may port it to other combos: #1250

@robwoolley robwoolley closed this Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants