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

use not deprecated console_bridge macros and undefine the deprecated ones #1149

Merged
merged 1 commit into from Aug 28, 2017

Conversation

dirk-thomas
Copy link
Member

Fixes #1145.

This patch avoids the side effect of defining these macro which are often subject to collisions in a ROS header.

@dirk-thomas dirk-thomas force-pushed the undefine_deprecated_console_bridge_macros branch from 8d74764 to 0e98226 Compare August 21, 2017 20:55
@@ -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_);
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

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?

Copy link
Member Author

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.

@dirk-thomas dirk-thomas force-pushed the undefine_deprecated_console_bridge_macros branch from a2be1c0 to 0e98226 Compare August 25, 2017 21:35
@dirk-thomas dirk-thomas merged commit ed8beb2 into lunar-devel Aug 28, 2017
@dirk-thomas dirk-thomas deleted the undefine_deprecated_console_bridge_macros branch August 28, 2017 16:40
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.

None yet

2 participants