Fix Travis config + issues (#47)

* cleanup .travis.yml
  - For Melodic, use source container as ROS underlay to avoid building MoveIt.
  - For Kinetic, build MoveIt master branch too.

* apply clang-tidy fixes
  - use const references where possible
  - simply return of booleans

* use compile-time switches to disable/enable code blocks

* clang-format

* fix remaining compiler warnings
  - order of member initialization
  - unused variables

Author: davetcoleman

.travis.yml

@@ -1,22 +1,26 @@
-# This config file for Travis CI utilizes https://github.com/ros-planning/moveit_ci/ package.
+# This config file for Travis CI utilizes https://github.com/ros-planning/moveit_ci package.
 sudo: required
-dist: trusty
+dist: xenial  # distro used by Travis, moveit_ci uses the docker image's distro
 services:
   - docker
 language: cpp
-cache: ccache
 compiler: gcc
+cache: ccache
 
-notifications:
-  email:
-    recipients:
-      - [email protected]
 env:
+  global:
+    - DOCKER_IMAGE=moveit/moveit:master-source
+    - CXXFLAGS="-Wall -Wextra -Wwrite-strings -Wunreachable-code -Wpointer-arith -Wredundant-decls -Wno-unused-parameter -Wno-unused-function"
+    - WARNINGS_OK=false
   matrix:
-    - ROS_DISTRO=kinetic ROS_REPO=ros              TEST=clang-format
-    - ROS_DISTRO=melodic ROS_REPO=ros              UPSTREAM_WORKSPACE=moveit.rosinstall
-    - ROS_DISTRO=melodic ROS_REPO=ros-shadow-fixed UPSTREAM_WORKSPACE=moveit.rosinstall
+    - TEST="clang-format catkin_lint"
+    - TEST=clang-tidy-fix
+    - DOCKER_IMAGE=moveit/moveit:melodic-source
+    - DOCKER_IMAGE=moveit/moveit:master-source
+    - DOCKER_IMAGE=moveit/moveit:kinetic-ci  UPSTREAM_WORKSPACE=moveit.rosinstall  TEST_BLACKLIST=moveit_ros_perception
+
 before_script:
-  - git clone -q https://github.com/ros-planning/moveit_ci.git .moveit_ci
+  - git clone -q --depth=1 https://github.com/ros-planning/moveit_ci.git .moveit_ci
+
 script:
-  - .moveit_ci/travis.sh
+  - .moveit_ci/travis.sh

include/moveit_visual_tools/imarker_end_effector.h

@@ -176,7 +176,7 @@ namespace
 {
 /** \brief Collision checking handle for IK solvers */
 bool isStateValid(const planning_scene::PlanningScene* planning_scene, bool verbose, bool only_check_self_collision,
-                  moveit_visual_tools::MoveItVisualToolsPtr visual_tools_, robot_state::RobotState* state,
+                  const moveit_visual_tools::MoveItVisualToolsPtr& visual_tools_, robot_state::RobotState* state,
                   const robot_state::JointModelGroup* group, const double* ik_solution);
 }
 

include/moveit_visual_tools/imarker_robot_state.h

@@ -98,7 +98,7 @@ class IMarkerRobotState
   bool saveToFile();
 
   /** \brief Set where in the parent class the feedback should be sent */
-  void setIMarkerCallback(IMarkerCallback callback);
+  void setIMarkerCallback(const IMarkerCallback& callback);
 
   /** \brief Get a pointer to the current robot state */
   moveit::core::RobotStateConstPtr getRobotState()
@@ -111,7 +111,7 @@ class IMarkerRobotState
   }
 
   /** \brief Set the robot state */
-  void setRobotState(moveit::core::RobotStatePtr state);
+  void setRobotState(const moveit::core::RobotStatePtr& state);
 
   /** \brief Set the robot state to current in planning scene monitor */
   void setToCurrentState();
@@ -138,7 +138,7 @@ class IMarkerRobotState
     return name_to_eef_[name];
   }
 
-  bool setFromPoses(const EigenSTL::vector_Isometry3d poses, const moveit::core::JointModelGroup* group);
+  bool setFromPoses(const EigenSTL::vector_Isometry3d& poses, const moveit::core::JointModelGroup* group);
 
 protected:
   // --------------------------------------------------------
@@ -186,7 +186,7 @@ namespace
 {
 /** \brief Collision checking handle for IK solvers */
 bool isIKStateValid(const planning_scene::PlanningScene* planning_scene, bool verbose, bool only_check_self_collision,
-                    moveit_visual_tools::MoveItVisualToolsPtr visual_tools_, robot_state::RobotState* state,
+                    const moveit_visual_tools::MoveItVisualToolsPtr& visual_tools_, robot_state::RobotState* state,
                     const robot_state::JointModelGroup* group, const double* ik_solution);
 }
 

include/moveit_visual_tools/moveit_visual_tools.h

@@ -428,7 +428,7 @@ class MoveItVisualTools : public rviz_visual_tools::RvizVisualTools
    * \brief Helper for publishCollisionWall
    */
   void getCollisionWallMsg(double x, double y, double z, double angle, double width, double height,
-                           const std::string name, moveit_msgs::CollisionObject& collision_obj);
+                           const std::string& name, moveit_msgs::CollisionObject& collision_obj);
 
   /**
    * \brief Publish a typical room wall
@@ -442,10 +442,10 @@ class MoveItVisualTools : public rviz_visual_tools::RvizVisualTools
    * \return true on sucess
    */
   bool publishCollisionWall(double x, double y, double angle = 0.0, double width = 2.0, double height = 1.5,
-                            const std::string name = "wall",
+                            const std::string& name = "wall",
                             const rviz_visual_tools::colors& color = rviz_visual_tools::GREEN);
   bool publishCollisionWall(double x, double y, double z, double angle = 0.0, double width = 2.0, double height = 1.5,
-                            const std::string name = "wall",
+                            const std::string& name = "wall",
                             const rviz_visual_tools::colors& color = rviz_visual_tools::GREEN);
 
   /**
@@ -468,7 +468,8 @@ class MoveItVisualTools : public rviz_visual_tools::RvizVisualTools
   }
 
   bool publishCollisionTable(double x, double y, double z, double angle, double width, double height, double depth,
-                             const std::string name, const rviz_visual_tools::colors& color = rviz_visual_tools::GREEN);
+                             const std::string& name,
+                             const rviz_visual_tools::colors& color = rviz_visual_tools::GREEN);
 
   /**
    * \brief Load a planning scene to a planning_scene_monitor from file
@@ -550,7 +551,7 @@ class MoveItVisualTools : public rviz_visual_tools::RvizVisualTools
   bool publishTrajectoryPath(const robot_trajectory::RobotTrajectoryPtr& trajectory, bool blocking = false);
   bool publishTrajectoryPath(const robot_trajectory::RobotTrajectory& trajectory, bool blocking = false);
   bool publishTrajectoryPath(const moveit_msgs::RobotTrajectory& trajectory_msg,
-                             const moveit::core::RobotStateConstPtr robot_state, bool blocking = false);
+                             const moveit::core::RobotStateConstPtr& robot_state, bool blocking = false);
   bool publishTrajectoryPath(const moveit_msgs::RobotTrajectory& trajectory_msg,
                              const moveit::core::RobotState& robot_state, bool blocking = false);
   bool publishTrajectoryPath(const moveit_msgs::RobotTrajectory& trajectory_msg,
@@ -568,7 +569,7 @@ class MoveItVisualTools : public rviz_visual_tools::RvizVisualTools
   bool publishTrajectoryLine(const moveit_msgs::RobotTrajectory& trajectory_msg,
                              const moveit::core::LinkModel* ee_parent_link, const robot_model::JointModelGroup* arm_jmg,
                              const rviz_visual_tools::colors& color = rviz_visual_tools::LIME_GREEN);
-  bool publishTrajectoryLine(const robot_trajectory::RobotTrajectoryPtr robot_trajectory,
+  bool publishTrajectoryLine(const robot_trajectory::RobotTrajectoryPtr& robot_trajectory,
                              const moveit::core::LinkModel* ee_parent_link,
                              const rviz_visual_tools::colors& color = rviz_visual_tools::LIME_GREEN);
   bool publishTrajectoryLine(const robot_trajectory::RobotTrajectory& robot_trajectory,
@@ -586,7 +587,7 @@ class MoveItVisualTools : public rviz_visual_tools::RvizVisualTools
   bool publishTrajectoryLine(const moveit_msgs::RobotTrajectory& trajectory_msg,
                              const robot_model::JointModelGroup* arm_jmg,
                              const rviz_visual_tools::colors& color = rviz_visual_tools::LIME_GREEN);
-  bool publishTrajectoryLine(const robot_trajectory::RobotTrajectoryPtr robot_trajectory,
+  bool publishTrajectoryLine(const robot_trajectory::RobotTrajectoryPtr& robot_trajectory,
                              const robot_model::JointModelGroup* arm_jmg,
                              const rviz_visual_tools::colors& color = rviz_visual_tools::LIME_GREEN);
   bool publishTrajectoryLine(const robot_trajectory::RobotTrajectory& robot_trajectory,
@@ -627,16 +628,16 @@ class MoveItVisualTools : public rviz_visual_tools::RvizVisualTools
    * \param color - how to highlight the robot (solid-ly) if desired, default keeps color as specified in URDF
    * \return true on success
    */
-  bool publishRobotState(const std::vector<double> joint_positions, const robot_model::JointModelGroup* jmg,
+  bool publishRobotState(const std::vector<double>& joint_positions, const robot_model::JointModelGroup* jmg,
                          const rviz_visual_tools::colors& color = rviz_visual_tools::DEFAULT);
 
   /**
    * \brief Publish a complete robot state to Rviz
    *        To use, add a RobotState marker to Rviz and subscribe to the DISPLAY_ROBOT_STATE_TOPIC, above
    * \param robot_state - joint values of robot
    * \param color - how to highlight the robot (solid-ly) if desired, default keeps color as specified in URDF
-   * \param highlight_links - if the |color| is not |DEFAULT|, allows selective robot links to be highlighted. by
-   * default (empty) all links are highlighted
+   * \param highlight_links - if the |color| is not |DEFAULT|, allows selective robot links to be highlighted.
+   * By default (empty) all links are highlighted.
    */
   bool publishRobotState(const moveit::core::RobotState& robot_state,
                          const rviz_visual_tools::colors& color = rviz_visual_tools::DEFAULT,
@@ -659,7 +660,7 @@ class MoveItVisualTools : public rviz_visual_tools::RvizVisualTools
    * \brief Print to console the current robot state's joint values within its limits visually
    * \param robot_state - the robot to show
    */
-  void showJointLimits(moveit::core::RobotStatePtr robot_state);
+  void showJointLimits(const moveit::core::RobotStatePtr& robot_state);
 
   /**
    * @brief Get the planning scene monitor that this class is using

moveit.rosinstall

@@ -1,17 +1,9 @@
 # This file is intended for users who want to build MoveIt from source.
 # Used with wstool, users can download source of all packages of MoveIt.
-- git:
-    local-name: moveit
-    uri: https://github.com/ros-planning/moveit.git
-    version: melodic-devel
 - git:
     local-name: moveit_msgs
     uri: https://github.com/ros-planning/moveit_msgs.git
     version: melodic-devel
-- git:
-    local-name: moveit_resources
-    uri: https://github.com/ros-planning/moveit_resources.git
-    version: master
 - git:
     local-name: rviz_visual_tools
     uri: https://github.com/PickNikRobotics/rviz_visual_tools.git
@@ -21,12 +13,6 @@
     uri: https://github.com/ros-planning/geometric_shapes.git
     version: melodic-devel
 - git:
-    local-name: srdfdom
-    uri: https://github.com/ros-planning/srdfdom.git
+    local-name: moveit
+    uri: https://github.com/ros-planning/moveit.git
     version: melodic-devel
-
-# TODO: remove:
-- git:
-    local-name: graph_msgs
-    uri: https://github.com/davetcoleman/graph_msgs.git
-    version: jade-devel

src/imarker_end_effector.cpp

@@ -56,12 +56,12 @@ IMarkerEndEffector::IMarkerEndEffector(IMarkerRobotState* imarker_parent, const
                                        ArmData arm_data, rviz_visual_tools::colors color)
   : name_(imarker_name)
   , imarker_parent_(imarker_parent)
+  , imarker_state_(imarker_parent_->imarker_state_)
   , psm_(imarker_parent_->psm_)
+  , visual_tools_(imarker_parent_->visual_tools_)
   , arm_data_(arm_data)
   , color_(color)
   , imarker_server_(imarker_parent_->imarker_server_)
-  , imarker_state_(imarker_parent_->imarker_state_)
-  , visual_tools_(imarker_parent_->visual_tools_)
 {
   // Get pose from robot state
   imarker_pose_ = imarker_state_->getGlobalLinkTransform(arm_data_.ee_link_);
@@ -108,7 +108,7 @@ void IMarkerEndEffector::iMarkerCallback(const visualization_msgs::InteractiveMa
   // Only allow one feedback to be processed at a time
   {
     // boost::unique_lock<boost::mutex> scoped_lock(imarker_mutex_);
-    if (imarker_ready_to_process_ == false)
+    if (!imarker_ready_to_process_)
     {
       return;
     }
@@ -276,26 +276,24 @@ IMarkerEndEffector::makeBoxControl(visualization_msgs::InteractiveMarker& msg)
 namespace
 {
 bool isStateValid(const planning_scene::PlanningScene* planning_scene, bool verbose, bool only_check_self_collision,
-                  moveit_visual_tools::MoveItVisualToolsPtr visual_tools, moveit::core::RobotState* robot_state,
+                  const moveit_visual_tools::MoveItVisualToolsPtr& visual_tools, moveit::core::RobotState* robot_state,
                   const moveit::core::JointModelGroup* group, const double* ik_solution)
 {
   // Apply IK solution to robot state
   robot_state->setJointGroupPositions(group, ik_solution);
   robot_state->update();
 
-  // Ensure there are objects in the planning scene
-  if (false)
+#if 0  // Ensure there are objects in the planning scene
+  const std::size_t num_collision_objects = planning_scene->getCollisionWorld()->getWorld()->size();
+  if (num_collision_objects == 0)
   {
-    const std::size_t num_collision_objects = planning_scene->getCollisionWorld()->getWorld()->size();
-    if (num_collision_objects == 0)
-    {
-      ROS_ERROR_STREAM_NAMED("cart_path_planner", "No collision objects exist in world, you need at least a table "
-                                                  "modeled for the controller to work");
-      ROS_ERROR_STREAM_NAMED("cart_path_planner", "To fix this, relaunch the teleop/head tracking/whatever MoveIt "
-                                                  "node to publish the collision objects");
-      return false;
-    }
+    ROS_ERROR_STREAM_NAMED("cart_path_planner", "No collision objects exist in world, you need at least a table "
+                           "modeled for the controller to work");
+    ROS_ERROR_STREAM_NAMED("cart_path_planner", "To fix this, relaunch the teleop/head tracking/whatever MoveIt "
+                           "node to publish the collision objects");
+    return false;
   }
+#endif
 
   if (!planning_scene)
   {
@@ -328,4 +326,4 @@ bool isStateValid(const planning_scene::PlanningScene* planning_scene, bool verb
   return false;
 }
 
-}  // end annonymous namespace
+}  // namespace