-
Notifications
You must be signed in to change notification settings - Fork 911
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
use not deprecated console_bridge macros and undefine the deprecated ones #1149
use not deprecated console_bridge macros and undefine the deprecated ones #1149
Conversation
14a7220
to
8d74764
Compare
8d74764
to
0e98226
Compare
@@ -569,7 +581,7 @@ void Bag::doWrite(std::string const& topic, ros::Time const& time, T const& msg, | |||
|
|||
// Check if we want to stop this chunk | |||
uint32_t chunk_size = getChunkOffset(); | |||
logDebug(" curr_chunk_size=%d (threshold=%d)", chunk_size, chunk_threshold_); | |||
CONSOLE_BRIDGE_logDebug(" curr_chunk_size=%d (threshold=%d)", chunk_size, chunk_threshold_); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, this doesn't work on Indigo. From what I can tell, /usr/include/console_bridge/console.h doesn't have the CONSOLE_BRIDGE_log*
macros on Indigo. I have an upcoming patch which I'll push here which defines them for the rosbag/bag.h
header file. It's not a generic solution (like having it in console_bridge/bridge.h
would be), but I'm not sure if we want to introduce that this late into Indigo. Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This patch is targeting lunar-devel
and there is no reason to backport this into Indigo. Therefore I don't think the second commit is necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, sorry, I missed that. I got confused because the initial bug report was for Indigo.
Alright, so for Lunar, I agree with your original patch (I'll revert mine off of this branch). However, that still leaves the original bug report about Indigo. It sounds like you want to just leave this be on Indigo; is that the case? If so, once we merge this patch I'll close the original bug report saying that it won't be fixed back in Indigo, but will be fixed going forward. Sound reasonable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It sounds like you want to just leave this be on Indigo; is that the case?
Yes, the version of console_bridge
in Indigo doesn't have the namespaces macros. Therefore I don't think every higher level package needs to work around this upstream problem.
The patch could be backported to Kinetic if the console_bridge
version on the platforms targeted by that support this.
a2be1c0
to
0e98226
Compare
Fixes #1145.
This patch avoids the side effect of defining these macro which are often subject to collisions in a ROS header.