Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix core warnings #2

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion moveit_core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@ if(NOT CMAKE_CXX_STANDARD)
endif()

# Warnings
# Remove -Wcast-qual to avoid warnings in tf2/LinearMath/Vector3.h.
henningkayser marked this conversation as resolved.
Show resolved Hide resolved
# TODO: Add -Wcast-qual back when PR(https://github.com/ros2/geometry2/pull/193) is merged.
add_compile_options(-Wall -Wextra
-Wwrite-strings -Wunreachable-code -Wpointer-arith -Wredundant-decls -Wcast-qual
-Wwrite-strings -Wunreachable-code -Wpointer-arith -Wredundant-decls
-Wno-unused-parameter -Wno-unused-function)
if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
# This too often has false-positives
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#include <moveit/collision_detection_fcl/collision_detector_fcl_plugin_loader.h>
#include <pluginlib/class_list_macros.h>
#include <pluginlib/class_list_macros.hpp>

namespace collision_detection
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ bool CollisionRobotDistanceField::getSelfCollisions(const collision_detection::C
}

RCLCPP_DEBUG(LOGGER_COLLISION_ROBOT_DISTANCE_FIELD,
("Self collision detected for link %s ", con.body_name_1).c_str());
"Self collision detected for link %s ", con.body_name_1.c_str());

con.body_type_2 = collision_detection::BodyTypes::ROBOT_LINK;
con.body_name_2 = "self";
Expand Down Expand Up @@ -890,7 +890,7 @@ DistanceFieldCacheEntryPtr CollisionRobotDistanceField::generateDistanceFieldCac
{
dfce->state_check_indices_.push_back(dfce->state_values_.size() - 1);
RCLCPP_DEBUG(LOGGER_COLLISION_ROBOT_DISTANCE_FIELD,
("Non-group joint %p will be checked for state changes", *name_iter).c_str());
"Non-group joint %p will be checked for state changes", (*name_iter).c_str());
}
}

Expand Down Expand Up @@ -1058,7 +1058,7 @@ void CollisionRobotDistanceField::addLinkBodyDecompositions(
{
RCLCPP_WARN(
LOGGER_COLLISION_ROBOT_DISTANCE_FIELD,
("Skipping model generation for link %s since it contains no geometries", link_models[i]->getName()).c_str());
"Skipping model generation for link %s since it contains no geometries", link_models[i]->getName().c_str());
continue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,7 @@ void CollisionWorldDistanceField::updateDistanceObject(const std::string& id, Di
else
{
RCLCPP_DEBUG(LOGGER_COLLISION_WORLD_DISTANCE_FIELD,
("Removing Object '%s' from CollisionWorldDistanceField", id).c_str());
"Removing Object '%s' from CollisionWorldDistanceField", id.c_str());
dfce->posed_body_point_decompositions_.erase(id);
}
}
Expand Down
2 changes: 1 addition & 1 deletion moveit_core/distance_field/test/test_distance_field.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ TEST(TestSignedPropagationDistanceField, TestPerformance)

dt = std::chrono::system_clock::now();
worstdfu.addPointsToField(bad_vec);
auto wd = std::chrono::system_clock::now() - dt;
std::chrono::duration<double> wd = std::chrono::system_clock::now() - dt;
printf("Time for unsigned adding %u uniform points is %g average %g\n", (unsigned int)bad_vec.size(), wd.count(),
wd.count() / (bad_vec.size() * 1.0));
dt = std::chrono::system_clock::now();
Expand Down
4 changes: 0 additions & 4 deletions moveit_core/kinematics_base/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@ set(MOVEIT_LIB_NAME moveit_kinematics_base)
add_library(${MOVEIT_LIB_NAME} SHARED src/kinematics_base.cpp)
set_target_properties(${MOVEIT_LIB_NAME} PROPERTIES VERSION "${${PROJECT_NAME}_VERSION}")

# Avoid warnings about deprecated members of KinematicsBase when building KinematicsBase itself.
# TODO: remove when deperecated variables (tip_frame_, search_discretization_) are finally removed.
# target_compile_options(${MOVEIT_LIB_NAME} PRIVATE -Wno-deprecated-declarations)

# This line is needed to ensure that messages are done being built before this is built
ament_target_dependencies(${MOVEIT_LIB_NAME} urdf urdfdom_headers)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,21 +328,6 @@ class KinematicsBase
virtual bool getPositionFK(const std::vector<std::string>& link_names, const std::vector<double>& joint_angles,
std::vector<geometry_msgs::msg::Pose>& poses) const = 0;

/**
* @brief Set the parameters for the solver, for use with kinematic chain IK solvers
* @param robot_description This parameter can be used as an identifier for the robot kinematics it is computed for;
* For example, the name of the ROS parameter that contains the robot description;
* @param group_name The group for which this solver is being configured
* @param base_frame The base frame in which all input poses are expected.
* This may (or may not) be the root frame of the chain that the solver operates on
* @param tip_frame The tip of the chain
* @param search_discretization The discretization of the search when the solver steps through the redundancy
*/
/* Replace by tip_frames-based method! */
MOVEIT_DEPRECATED virtual void setValues(const std::string& robot_description, const std::string& group_name,
const std::string& base_frame, const std::string& tip_frame,
double search_discretization);

/**
* @brief Set the parameters for the solver, for use with non-chain IK solvers
* @param robot_description This parameter can be used as an identifier for the robot kinematics it is computed for;
Expand All @@ -357,24 +342,6 @@ class KinematicsBase
const std::string& base_frame, const std::vector<std::string>& tip_frames,
double search_discretization);

/**
* @brief Initialization function for the kinematics, for use with kinematic chain IK solvers
* @param robot_description This parameter can be used as an identifier for the robot kinematics it is computed for;
* For example, the name of the ROS parameter that contains the robot description;
* @param group_name The group for which this solver is being configured
* @param base_frame The base frame in which all input poses are expected.
* This may (or may not) be the root frame of the chain that the solver operates on
* @param tip_frame The tip of the chain
* @param search_discretization The discretization of the search when the solver steps through the redundancy
* @return True if initialization was successful, false otherwise
*
* Instead of this method, use the method passing in a RobotModel!
* Default implementation returns false, indicating that this API is not supported.
*/
MOVEIT_DEPRECATED virtual bool initialize(const std::string& robot_description, const std::string& group_name,
const std::string& base_frame, const std::string& tip_frame,
double search_discretization);

/**
* @brief Initialization function for the kinematics, for use with non-chain IK solvers
* @param robot_description This parameter can be used as an identifier for the robot kinematics is computed for;
Expand Down Expand Up @@ -589,12 +556,6 @@ class KinematicsBase
std::string base_frame_;
std::vector<std::string> tip_frames_;

// The next two variables still exists for backwards compatibility
// with previously generated custom ik solvers like IKFast
// Replace tip_frame_ -> tip_frames_[0], search_discretization_ -> redundant_joint_discretization_
MOVEIT_DEPRECATED std::string tip_frame_;
MOVEIT_DEPRECATED double search_discretization_;

double default_timeout_;
std::vector<unsigned int> redundant_joint_indices_;
std::map<int, double> redundant_joint_discretization_;
Expand Down
42 changes: 5 additions & 37 deletions moveit_core/kinematics_base/src/kinematics_base.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ void KinematicsBase::storeValues(const moveit::core::RobotModel& robot_model, co
for (const std::string& name : tip_frames)
tip_frames_.push_back(removeSlash(name));
setSearchDiscretization(search_discretization);
search_discretization_ = search_discretization;
}

void KinematicsBase::setValues(const std::string& robot_description, const std::string& group_name,
Expand All @@ -77,50 +76,23 @@ void KinematicsBase::setValues(const std::string& robot_description, const std::
for (const std::string& name : tip_frames)
tip_frames_.push_back(removeSlash(name));
setSearchDiscretization(search_discretization);

// store deprecated values for backwards compatibility
search_discretization_ = search_discretization;
if (tip_frames_.size() == 1)
tip_frame_ = tip_frames_[0];
else
tip_frame_.clear();
}

void KinematicsBase::setValues(const std::string& robot_description, const std::string& group_name,
const std::string& base_frame, const std::string& tip_frame,
double search_discretization)
{
setValues(robot_description, group_name, base_frame, std::vector<std::string>({ tip_frame }), search_discretization);
}

bool KinematicsBase::initialize(const std::string& robot_description, const std::string& group_name,
const std::string& base_frame, const std::string& tip_frame,
double search_discretization)
{
return false; // default implementation returns false
}

bool KinematicsBase::initialize(const std::string& robot_description, const std::string& group_name,
const std::string& base_frame, const std::vector<std::string>& tip_frames,
double search_discretization)
{
// For IK solvers that do not support multiple tip frames, fall back to single pose call
if (tip_frames.size() == 1)
{
return initialize(robot_description, group_name, base_frame, tip_frames[0], search_discretization);
}

RCLCPP_ERROR(LOGGER_KINEMATICS_BASE, "This solver does not support multiple tip frames");
RCLCPP_ERROR(LOGGER_KINEMATICS_BASE, "IK plugin for group '%s' relies on deprecated API.",
group_name.c_str());
return false;
}

bool KinematicsBase::initialize(const moveit::core::RobotModel& robot_model, const std::string& group_name,
const std::string& base_frame, const std::vector<std::string>& tip_frames,
double search_discretization)
{
RCLCPP_WARN(LOGGER_KINEMATICS_BASE, "IK plugin for group '%s' relies on deprecated API. "
"Please implement initialize(RobotModel, ...).",
group_name.c_str());
RCLCPP_ERROR(LOGGER_KINEMATICS_BASE, "IK plugin for group '%s' relies on deprecated API.",
group_name.c_str());
return false;
}

Expand Down Expand Up @@ -175,11 +147,7 @@ bool KinematicsBase::supportsGroup(const moveit::core::JointModelGroup* jmg, std
}

KinematicsBase::KinematicsBase()
: tip_frame_("DEPRECATED")
// help users understand why this variable might not be set
// (if multiple tip frames provided, this variable will be unset)
, search_discretization_(DEFAULT_SEARCH_DISCRETIZATION)
, default_timeout_(DEFAULT_TIMEOUT)
:default_timeout_(DEFAULT_TIMEOUT)
{
supported_methods_.push_back(DiscretizationMethods::NO_DISCRETIZATION);
}
Expand Down
1 change: 1 addition & 0 deletions moveit_core/package.xml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
-->

<export>
<build_type>ament_cmake</build_type>
<moveit_core plugin="${prefix}/collision_detector_fcl_description.xml"/>
</export>
</package>
6 changes: 5 additions & 1 deletion moveit_core/profiler/src/profiler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,15 @@
#include <vector>
#include <algorithm>
#include <sstream>
#include <rclcpp/rclcpp.hpp>

namespace moveit
{
namespace tools
{

rclcpp::Logger LOGGER_MOVEIT_PROFILER = rclcpp::get_logger("moveit_profiler");

Profiler& Profiler::instance()
{
static Profiler p(false, false);
Expand Down Expand Up @@ -174,7 +178,7 @@ void Profiler::console()
std::stringstream ss;
ss << std::endl;
status(ss, true);
RCUTILS_LOG_INFO("profiler", ss.str().c_str());
RCLCPP_INFO(LOGGER_MOVEIT_PROFILER, ss.str().c_str());
}

/// @cond IGNORE
Expand Down