ADVRHumanoids / iit-centauro-ros-pkg

2 stars 0 forks source link

Refactor of robot repositories #18

Open alaurenzi opened 2 years ago

alaurenzi commented 2 years ago

@liesrock @DavideAntonucci @EdoardoRomiti Hi all, I am doing some work to improve our workflow concerning robot URDF/SRDF/capsules generation, and simulation, too.

Why

The weak points of our previous infrastructure, in my opinion, are the following: 1) one single big bash script doing everything and every time it is invoked

How

I implemented some solutions inside a refactor branch. Key points are: 1) basic structure does not change (all xacros stay where they are) 2) main xacro must support an additional argument config with a default value pointing to a default configuration file 3) cmake-based xacro generation with single targets for every config file; benefits are:

Usage

1) cloning the repository yields a ready-to-use package, generation is not required 5) make install support in case the source folder is not part of ROS_PACKAGE_PATH 2) from a developer's perspective, generation must be performed with a make <target-name> from the build folder, such as

Feedback

I'd like to hear some feedback from you before going further ahead with this project. Do you see a better way to accomplish some of the goals? Do you see further shortcomings of the current architecture that can be solved?

@EdoardoRomiti I am not sure how and if this affects you work (generation not from xacros but from discovered topology)

EdoardoRomiti commented 2 years ago

Personally I like many of the ideas here. With modular we follow a slightly different workflow for urdf/srdf generation than with Centauro, but I would say we have the same weakness in the Why section (apart from 1,4 and 5):

DavideAntonucci commented 2 years ago

Hi @alaurenzi,

Thanks a lot that you've involved me in this discussion. Personally, I really like the approach described before to create and maintain the URDF (Usage points 3 and 4). Another good point is Debian packaging creation that I've already used and it works.

The HOW points (4,5 and 6) are very useful for me when I want to play immediately with the robot.

I don't see weakness points using this approach right now.

alaurenzi commented 2 years ago

@nkashiri give this deb package a try Also check if the issues you encountered are being addressed here

nkashiri commented 2 years ago

following the installation of the deb file, and sourcing the bashrc, it is needed to ~$ rospack profile so that the robot can be simulated via ~$ roslaunch centauro_gazebo centauro_world.launch

nkashiri commented 2 years ago

There are these warnings:

[INFO] [1637584501.179652, 0.105500]: Calling service /gazebo/spawn_urdf_model Warning [parser_urdf.cc:1115] multiple inconsistent exists due to fixed joint reduction overwriting previous value [Gazebo/Residential] with [Gazebo/BlackTransparent]. Warning [parser_urdf.cc:1115] multiple inconsistent exists due to fixed joint reduction overwriting previous value [Gazebo/Residential] with [Gazebo/BlackTransparent]. Warning [parser_urdf.cc:1115] multiple inconsistent exists due to fixed joint reduction overwriting previous value [Gazebo/Residential] with [Gazebo/BlackTransparent]. Warning [parser_urdf.cc:1115] multiple inconsistent exists due to fixed joint reduction overwriting previous value [Gazebo/Residential] with [Gazebo/BlackTransparent].

Solution: when removing the wheels material line from centauro.gazebo.xacro, the warning is removed

nkashiri commented 2 years ago

Bonus point! it would be nice if the robot would fall on the ground from a shorter height.

liesrock commented 2 years ago

Grande @alaurenzi, I try to answer step by step:

Why

The weak points of our previous infrastructure, in my opinion, are the following:

  1. one single big bash script doing everything and every time it is invoked

    • generate urdf and srdf for all existing config files (from urdf/config/*.urdf.xacro)
    • generate capsule models
  2. does not report errors (or exit on error)
  3. does the generation even though no file has actually changed
  4. generates intermediate files inside the source tree (thus polluting it)
  5. unclear which files are generated (byproducts of generation) and which are actual source files
  6. no unit tests and continuous integration
  7. no debian generation
  8. issues with gazebo11 due to unsupported floating joint

The motivation are clear and the issues in all the reported points should be tackled.

How

I implemented some solutions inside a refactor branch. Key points are:

  1. basic structure does not change (all xacros stay where they are)
  2. main xacro must support an additional argument config with a default value pointing to a default configuration file
  3. cmake-based xacro generation with single targets for every config file; benefits are:

    • generation skipped if source files haven't changed
    • files are generated inside the build tree and published to the source tree with an explicit command
    • errors causes early exit
  4. dropped support for drag-and-drop simulation in gazebo
  5. perception simulation with additional arguments to be provided to the launch file (realsense:=true velodyne:=true)
  6. a basic integration test demonstrating that simulation is spawned successfully and xbot2 can start correctly
  7. Travis CI
  8. cpack based deb generation (example here)
  9. conditional inclusion of a floating joint (just for control)

Point 3 is great! Actually I would keep the support for drag-and-drop simulation in gazebo (point 4): why this choice? This can be useful (ROS-free) and easier to debug in my experience.

For the point 5 I would like to have also @aled96 feedback.

6,7 are super, as well as the debian generation (point 8) which we already used successfully on CENTAURO control pc.

aled96 commented 2 years ago

Nice work !

  1. perception simulation with additional arguments to be provided to the launch file (realsense:=true velodyne:=true)

For the point 5, I think it can be very useful to enable the cameras like this in order to speed up the customization of the robot sensors without the need to create the urdf again. It is also useful because we have to consider that the realsense d435i slows down the simulation and so it can be better to remove it if not needed.

alaurenzi commented 2 years ago

Point 3 is great! Actually I would keep the support for drag-and-drop simulation in gazebo (point 4): why this choice? This can be useful (ROS-free) and easier to debug in my experience.

The rationale for this choice is twofold 1) running gazebo and then dropping one of our robots is not enough for a correct simulation, since some WorldPlugins would be missing (namely the one for syncing to simulated clock); we would need a gazebo wrapper a la gazebo_ros 2) oftentimes (and for sure in the case of centauro), dropping it into a default world results in a very unstable simulation due to the default value of integration step size (1 ms)

I agree it is useful to have, but requires a bit of additional work that we could tackle at a second time

liesrock commented 2 years ago

Ok I see, it was a plus in the past, but probably no one is using it in a drag-and-drop fashion in these days: the points you mentioned are clear.

aled96 commented 2 years ago

For what concerns the velodyne and the realsenses, setting to true the corresponding arguments in the launch file allows to see in gazebo the robot with the sensors (since it uses the xacro), but when we launch xbot the sensors frames are not published since xbot takes as input the urdf (which currently does not contain the cameras).

I copy here also the links of the sensors xacros, if needed:

alaurenzi commented 2 years ago

To address this I will generate all urdfs with perception flags set to true. However, this currently means gazebo-specific plugins are also included inside the urdf files (in addition to the sensor-related frames).

This is a bit dirty in my opinion. The only solution I see is to modify the macros _t265, _d435i, and VLP-16 so to expose a parameter that avoids the inclusion of plugins.

Would this work @aled96 ? Do you see any drawback ?

aled96 commented 2 years ago

I think that it can be done without much problems. For the realsenses we have the xacro file with the links/joints and then the inclusion of the gazebo.xacro, so with an additional parameter we can simply avoid to include the plugin.

For the velodyne we have a single xacro file but, of course, we can still add a xacro:if clause to avoid considering the gazebo part.

I tried removing the gazebo inclusion from the D435i xacro file and there are no errors, we can see the model of the camera in the robot but no topics are published.