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

logging macros will not take a const Logger #679

Closed
wjwwood opened this issue Apr 2, 2019 · 4 comments
Closed

logging macros will not take a const Logger #679

wjwwood opened this issue Apr 2, 2019 · 4 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@wjwwood
Copy link
Member

wjwwood commented Apr 2, 2019

Right now if you try to pass a const rclcpp::Logger to a logging macro you'll get an error like this (taken from moveit/moveit2#21 (comment)):

/Users/victor/ros2_ws/install/rclcpp/include/rclcpp/logging.hpp:411:3: note: expanded from macro 'RCLCPP_ERROR'
  static_assert( \
  ^
/Users/victor/ros2_moveit_ws/src/moveit2/moveit_core/robot_state/src/conversions.cpp:100:9: error: static_assert failed due to requirement '::std::is_same<typename std::remove_reference<decltype(LOGGER)>::type, typename ::rclcpp::Logger>::value' "First argument to logging macros must be an rclcpp::Logger"
        RCLCPP_ERROR(LOGGER, "Caught %s", ex.what());
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

We seem to remove the reference but not the const qualifier when comparing with std::is_same.

@wjwwood wjwwood added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers labels Apr 2, 2019
@wjwwood wjwwood self-assigned this Apr 2, 2019
@Karsten1987
Copy link
Contributor

Can this be closed now that #680 is merged?

@wjwwood
Copy link
Member Author

wjwwood commented Apr 25, 2019

Yup.

@wjwwood wjwwood closed this as completed Apr 25, 2019
@Karsten1987
Copy link
Contributor

I believe this is still not correctly solved:

bool log_function(const rclcpp::Logger & logger)
{
  RCLCPP_INFO(logger, "successful log");
  return true;
}

TEST_F(TestLoggingMacros, test_log_from_node) {
  auto logger = rclcpp::get_logger("test_logging_logger");
  bool success = log_function(logger);
  EXPECT_TRUE(success);
}

yields to

/Users/karsten/workspace/osrf/ros2_full/src/ros2/rclcpp/rclcpp/test/test_logging.cpp:168:3: error: static_assert failed due to requirement '::std::is_same<typename std::remove_reference<typename std::remove_cv<decltype(logger)>::type>::type, typename ::rclcpp::Logger>::value' "First argument to logging macros must be an rclcpp::Logger"
  RCLCPP_INFO(logger, "successful log");
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/Users/karsten/workspace/osrf/ros2_full/build/rclcpp/include/rclcpp/logging.hpp:214:5: note: expanded from macro 'RCLCPP_INFO'
    static_assert( \
    ^
1 error generated.

@Karsten1987 Karsten1987 reopened this Aug 8, 2019
@emersonknapp
Copy link
Collaborator

@Karsten1987 was this issue resolved by #820 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants