search

ros / meta-ros

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

{rolling} fixing orocos-kdl-vendor #1118

Closed DasRoteSkelett closed 7 months ago

DasRoteSkelett commented 7 months ago

The recipe was failing with

Fetcher failure: Unable to find revision f1bbaed1b5190dbfefd08ca445e53d20d1a0066e in branch release/rolling/orocos_kdl_vendor

This comes from the patch that will be dependent on the hashes of the package, therefore this approach is prone to fail on every refresh of this package.

The alternative approach is to introduce a yocto recipe for orocos-kdl, which basically makes any appends to orocos-kdl-vendor obsolete.

orocos-kdl can be used in other distros (humble, noetic) as well, so I placed it into ros-common layer.

robwoolley commented 7 months ago

Hi @DasRoteSkelett

Thanks for submitting this. I spent a few hours looking at it. Some things I noticed:

The orocos-kdl version tends to be tied to the ROS release. I think this means that the orocos-kdl and python3-pykdl recipes should be in the meta-ros2- or at least meta-ros2 so it doesn't impact noetic.

I also noticed that the PyKDL.so file was getting installed to /usr/lib/python3.12/dist-packages, which is a Debian-ism. For Yocto we should probably use site-packages.

I had some difficulty with python-orocos-kdl-vendor finding PyKDL.so. It wants to run Python to find it which naturally doesn't work for a cross-build environment.

I agree with your comment about the hardcoded commit id being prone to failing every time. The concern I have is that the vendor package specifies a given commit id so we would need to make sure to keep the orocos-kdl package up-to-date with that commit id every time we regenerate the recipe: https://github.com/ros2-gbp/orocos_kdl_vendor-release/blob/release/rolling/orocos_kdl_vendor/CMakeLists.txt#L13

Here are the changes that I built upon the work you did: https://github.com/ros/meta-ros/commit/02458920110d6ca26bffffde432979035eaa0a5b

It builds fine, however I'm not entirely satisfied due to the aforementioned commit id concern I mentioned above.

What are your thoughts on the changes and how to avoid fixing up the commit ids?

DasRoteSkelett commented 7 months ago

Closing in favor of #1142