whole_body_state_rviz_plugin icon indicating copy to clipboard operation
whole_body_state_rviz_plugin copied to clipboard

Improvement suggestions

Open VladimirIvan opened this issue 3 years ago • 3 comments

  • README.md recommend using catkin_tools (catkin build command) instead of the deprecated catkin_make.
  • package.xml unspecified version. Verion 2 or 3 is now supported on all platforms.
  • package.xml doesn't list pinocchio as a dependency. Check if the ROS binaries are working with the plugin.
  • Use #pragma once instead of the #ifdef guard. All modern compilers support it now (GCC3.4+).
  • Include a gif with example trajectory/state visualisation in the readme, it will attract more users.
  • Consider writing some documentation with examples or linking them here if they are in another package.
  • Since the code is compiled into a plugin, catkin_package() can be left empty in the cmakelist. The headers also don't need to be installed. This will prevent users accidentally linking against the plugin, which may cause problems.
  • /** ... **/ type doxygen comments annotate the following line. The header files use these to describe a group of variables but only the first variable will contain the description. Either use the member groups feature or just switch them to regular comments.

The code is well structured and documented. Well done.

VladimirIvan avatar Aug 05 '20 11:08 VladimirIvan