icub-tech-iit / xcub-moveit2

Collect the outcomes of our study on the use of MoveIT with our robots
BSD 3-Clause "New" or "Revised" License
3 stars 0 forks source link

Last developments on controlling yarp-based robots with MoveIt2 #16

Closed martinaxgloria closed 10 months ago

martinaxgloria commented 10 months ago

With this PR, I would like to include all the developments done with this activity so far. In particular:

After that, @Nicogene or anyone else could try to install it by following the README.md. In this sense, I'll be able to understand if I missed something or if there's something not so clear.

martinaxgloria commented 10 months ago

It is not clear to me the cmake structure, it seems that there are "multiple" root CMakeLists If it is something needed by colcon it is fine otherwise I will add a main cmakelist and then invoke the others with add_subdirectory

When you create a ros2 package, automatically it creates a folder with its own package.xml and CMakeLists.txt files besides the src folder. I think this is due to the fact that each package inside the ros workspace could be compiled separately from the others.

martinaxgloria commented 10 months ago

As the ros people suggest, it's better to have a src folder inside your workspace and create each ros package in there in order to keep the top level of the workspace as clean as possible (see here for example)

traversaro commented 10 months ago

As the ros people suggest, it's better to have a src folder inside your workspace and create each ros package in there in order to keep the top level of the workspace as clean as possible (see here for example)

Just as a general comment, having multiple CMake projects in the same repo has some advantages, especially when using ros tooling such as colcon. On the other hand, it is quite uncommon for C++/CMake projects outside of the ros world/bubble to have a single repo with multiple CMake projects, and in general it may complexify consuming the software in the repo if one is not using colcon. For example, let's say that we want to add this repo to a CMake-based superbuild, such as the robotology-superbuild. Technically, we should add a single Build<package>.cmake script for each CMake package in the repo (see https://github.com/robotology/robotology-superbuild/blob/master/doc/developers-faqs.md#how-to-add-a-new-package). So, "better" is always a bit relative as term. : )

martinaxgloria commented 10 months ago

Hi @traversaro, thanks for your clarification! You're right, I used a misleading word since my reply was strictly related to the ros world, not in general of course

martinaxgloria commented 10 months ago

About the testing we agreed to add a CI job in order to compile it and run the test(?) on a clean machine

About that, how will we handle the dependencies? I mean, to compile this repo in a clean environment it's necessary to install all the needed ros2 packages, moveit2, install and compile yarp-devices-ros2, and so on. To test whether the packages contained in this repo will compile without errors, how about starting from a pre-built environment with at least ros2 and moveit2 already installed? Or do you think it's better to compile them every time the action is triggered?

cc @Nicogene @pattacini @traversaro

pattacini commented 10 months ago

To test whether the packages contained in this repo will compile without errors, how about starting from a pre-built environment with at least ros2 and moveit2 already installed? Or do you think it's better to compile them every time the action is triggered?

This is doable and I do that for the robots-configuration CI via a docker container. Relevant resources:

At any rate, I would deal with the CI once the repo is public. We will need to do several tests and thus better off going with the free actions minutes quota.

pattacini commented 10 months ago

This is doable and I do that for the robots-configuration CI via a docker container.

But, perhaps, there are already containers maintained by the community that make available ROS and MoveIT.

traversaro commented 10 months ago

One possible way is to compile the dependency and then cache them, se https://github.com/ami-iit/bipedal-locomotion-framework/blob/master/.github/workflows/ci.yml#L155 . However, probably I am missing a point: how do we expect downstream users to consume this software? We want to rename this repo from study- to a generic software repo, or something else?

pattacini commented 10 months ago

We want to rename this repo from study- to a generic software repo, or something else?

Yep.

pattacini commented 10 months ago

One possible way is to compile the dependency and then cache them, se https://github.com/ami-iit/bipedal-locomotion-framework/blob/master/.github/workflows/ci.yml#L155 .

This is another way of doing the same thing 👍🏻

traversaro commented 10 months ago

One possible way is to compile the dependency and then cache them, se https://github.com/ami-iit/bipedal-locomotion-framework/blob/master/.github/workflows/ci.yml#L155 .

This is another way of doing the same thing 👍🏻

Yes, sorry I replied before you posted your reply.

pattacini commented 10 months ago

Yes, sorry I replied before you posted your reply.

No problem, I meant only to report that we have multiple choices at hand 👍🏻

martinaxgloria commented 10 months ago

This is doable and I do that for the robots-configuration CI via a docker container.

This is what I was referring to :+1:

At any rate, I would deal with the CI once the repo is public. We will need to do several tests and thus better off going with the free actions minutes quota.

Ok, we could account for this later on.

martinaxgloria commented 10 months ago

One possible way is to compile the dependency and then cache them, se https://github.com/ami-iit/bipedal-locomotion-framework/blob/master/.github/workflows/ci.yml#L155 .

Thanks for your suggestion @traversaro!

martinaxgloria commented 10 months ago

As discussed, I removed the test automatically generated and I updated the README.md with a use case on how to run the demo we showed during the PI review

cc @pattacini @Nicogene

pattacini commented 10 months ago

Great!