Add GHA CI & Docker

[rhaschke]

Adds GitHub Actions for creation of Docker images with pre-installed MuJoCo versions, and CI build and testing with CodeCov

Todos

• Github Apps:
  • [ ] Install behavior bot for this repo (https://github.com/behaviorbot/welcome)
  • [ ] Install CodeCov for this repo (https://github.com/apps/codecov)
• Docker
  • [x] Change dockerhub user
  • [x] Add dockerhub secrets
• Formatting ~Apply formatting result patch to PR~
• Docs
  • [x] Change README.md badge links
• PR Rules
  • [ ] Require successful Format and CI Checks

[codecov-commenter]

Welcome to Codecov :tada:

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered :open_umbrella:

[rhaschke]

I had a hard time finding a compiler error only occurring with clang and only on GHA, but not locally (first). Eventually, clang uses C++14 by default, while gcc already uses C++17, which is required by ROS/log4cxx.

Now, there are only real issues in the source code, revealed by the more picky clang compiler... @DavidPL1, I hand over to you.

I suggest to squash-merge this PR if ready.

[DavidPL1]

Any idea, why some unit tests got broken?

I'm not really sure yet. The last commit at least fixes _mujoco_ros_plugintest on focal (with a catkin workspace configured to use the installspace) when run locally with

roslaunch mujoco_ros_pkgs/mujoco_ros/test/launch/mujoco_ros_plugin.test 
devel/.private/mujoco_ros/lib/mujoco_ros/mujoco_ros_plugin_test

in two separate terminals.

However, running catkin test mujoco_ros still fails without any helpful information:

[Testcase: testmujoco_ros_plugin_test] ... FAILURE!
FAILURE: False is not true : test [mujoco_ros_plugin_test] did not generate test results

And running colcon test gives the same output.

[DavidPL1]

Looks like were almost done.

The remaining clang-tidy errors now come from lodepng which currently is just a 1:1 copy of the original upstream header and implementation.

There are two ways to handle this: Either we stick to the current version, apply the fixes and track upstream changes into our version (there haven't been many changes in the past year (commit-history)) or we switch to using CMake's FetchContent like deepmind does to fetch and build a specific commit upon building, and disable clang-tidy checks for the fetched contents.

What do you think, @rhaschke?

[rhaschke]

Either we stick to the current version, apply the fixes, and track upstream changes into our version ...

Is there a special reason to use lodepng instead of libpng? According to this blog, the latter is much faster. In general, I don't care about the way to go. I think adapting the imported files is the easiest.

[DavidPL1]

Either we stick to the current version, apply the fixes, and track upstream changes into our version ...

Is there a special reason to use lodepng instead of libpng? According to this blog, the latter is much faster. In general, I don't care about the way to go. I think adapting the imported files is the easiest.

There is no specific reason other than using the same library as deepmind does in their simulator program. They also don't explain their choice. The test you linked is rather old (2015) and the more recent comments don't mention newer results for lodepng. Additionally, the benchmark only covers image decoding.

In this case lodepng is only used to write a PNG from a raw RGB buffer when clicking the screenshot button, which is also not expected to happen too often. Keeping the library thin seems to be prioritized over performance here.

[DavidPL1]

Looks good. I think you can squash-merge 🥇

I'll prepare the other plugins outside of this repo to comply with the API changes and then squash-merge the PR. Thanks for your help, especially with the workflows!

[rhaschke]

You are welcome.