Skip to content

Fix logging for updated rclcpp interface#303

Merged
mjcarroll merged 4 commits into
masterfrom
fix_log_update
Dec 22, 2020
Merged

Fix logging for updated rclcpp interface#303
mjcarroll merged 4 commits into
masterfrom
fix_log_update

Conversation

@mjcarroll

@mjcarroll mjcarroll commented Dec 21, 2020

Copy link
Copy Markdown
Member

This has caused failures in the nightly jobs on platforms where ros1_bridge is built.

I believe that it is related to this PR: ros2/rclcpp#1442

@audrow Can you confirm that this was an expected side effect?

Signed-off-by: Michael Carroll <michael@openrobotics.org>
Comment thread src/bridge.cpp Outdated
Signed-off-by: Michael Carroll <michael@openrobotics.org>

Co-authored-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Comment thread src/bridge.cpp Outdated

@audrow audrow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think Clang will raise a warning with the change that's been made. I've suggested two alternatives.

Comment thread src/bridge.cpp Outdated
@audrow

audrow commented Dec 21, 2020

Copy link
Copy Markdown
Member

Also, this is an expected side effect. After ros2/rclcpp#1442, the logging macros only accept char *s and the format argument should be a string literal.

Signed-off-by: Michael Carroll <michael@openrobotics.org>

@audrow audrow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, with green CI!

@mjcarroll

Copy link
Copy Markdown
Member Author
  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll

mjcarroll commented Dec 22, 2020

Copy link
Copy Markdown
Member Author

This actually requires the ci_packaging job, since it's ros1_bridge:

Build Status

@mjcarroll
mjcarroll merged commit 08cf32c into master Dec 22, 2020
@delete-merged-branch
delete-merged-branch Bot deleted the fix_log_update branch December 22, 2020 16:07
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.

3 participants