Last developments on controlling yarp-based robots with MoveIt2 #16
Conversation
Remove direct dependencies to icub Update README.md Changes needed to run moveit task constructor Cartesian control torso+left_arm Add first attempt to compute circular trajectory Update circular cartesian trajectory and remove check links collision Demo further improvements Demo iCubGenova11 Fix Fix end effector reference frame Demo PI16 Update documentation and remove not usefull things Update README.md Update .gitignore Update README.md
martinaxgloria
force-pushed
the
feat/ergocub
branch
from
79f7872 to
e0e862b
Compare
There was a problem hiding this comment.
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 would add a main CMakeLists and then invoke the others with add_subdirectory
About the testing we agreed to add a CI job in order to compile it and run the test(?) on a clean machine
| @@ -34,7 +34,7 @@ TEST(TestLoadICubController, load_controller) | |||
|
|
|||
| ASSERT_NE( | |||
| cm.load_controller( | |||
| "test_icub_controller", "icub_controller/ICubController"), | |||
| "test_robot_controller", "robot_controller/RobotController"), | |||
There was a problem hiding this comment.
This was a basic test (to be honest I've never tested it) that could be useful to check if the controller could be found and loaded. It was included during the hardware interface implementation, I'll try to run and implement it to be used as a test
There was a problem hiding this comment.
Obv after your changes, thanks!
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. |
|
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 |
|
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 |
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? |
This is doable and I do that for the
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. |
But, perhaps, there are already containers maintained by the community that make available ROS and MoveIT. |
|
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 |
Yep. |
This is another way of doing the same thing 👍🏻 |
Yes, sorry I replied before you posted your reply. |
No problem, I meant only to report that we have multiple choices at hand 👍🏻 |
This is what I was referring to 👍
Ok, we could account for this later on. |
Thanks for your suggestion @traversaro! |
|
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 |
|
Great! |
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.