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

Allow logger to be passed into macros as a reference #423

Merged
merged 2 commits into from
Dec 5, 2017

Conversation

dhood
Copy link
Member

@dhood dhood commented Dec 5, 2017

Otherwise when captured by a lambda as in https://github.com/ros2/demos/pull/205/files#diff-f3c8fd029db528b5408e0a785999636eR135 it does not pass the rclcpp::Logger type check because it is rclcpp::Logger &

CI windows including the logger usage in ros2/demos#205: Build Status (image_tools testing passed)

This is the type when captured by reference in a lambda; windows can't
resolve it without.
@dhood dhood added the in review Waiting for review (Kanban column) label Dec 5, 2017
@dhood dhood self-assigned this Dec 5, 2017
@dhood dhood requested a review from wjwwood December 5, 2017 22:22
Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, 👍

@@ -85,7 +85,7 @@ def is_supported_feature_combination(feature_combination):
* \param ... The format string, followed by the variable arguments for the format string
*/
#define RCLCPP_@(severity)@(suffix)(logger, @(''.join([p + ', ' for p in get_macro_parameters(feature_combination).keys()]))...) \
static_assert(::std::is_same<decltype(logger), ::rclcpp::Logger>::value, "First argument to logging macros must be an rclcpp::Logger"); \
static_assert(::std::is_same<std::remove_reference<decltype(logger)>::type, ::rclcpp::Logger>::value, "First argument to logging macros must be an rclcpp::Logger"); \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Line is looking very long :)

@dhood dhood merged commit d823982 into master Dec 5, 2017
@dhood dhood deleted the fix_windows_logger_ref branch December 5, 2017 23:12
@dhood dhood removed the in review Waiting for review (Kanban column) label Dec 5, 2017
nnmm pushed a commit to ApexAI/rclcpp that referenced this pull request Jul 9, 2022
Signed-off-by: Abby Xu <abbyxu@amazon.com>
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Aug 5, 2022
Signed-off-by: Karsten Knese <karsten@openrobotics.org>
DensoADAS pushed a commit to DensoADAS/rclcpp that referenced this pull request Aug 5, 2022
* export shared_queues_vendor

Signed-off-by: Knese Karsten <karsten@openrobotics.org>

* Revert "find rosbag2_cpp (tinyxml2) before rcl (ros2#423)"

This reverts commit 48fd15e.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants