Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

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

Merged
dirk-thomas merged 1 commit intolunar-develfrom
undefine_deprecated_console_bridge_macros
Aug 28, 2017
Merged

use not deprecated console_bridge macros and undefine the deprecated ones#1149
dirk-thomas merged 1 commit intolunar-develfrom
undefine_deprecated_console_bridge_macros

Conversation

@dirk-thomas
Copy link
Copy Markdown
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
// 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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants