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

How to really use rosconsole_bridge? #12

Open
HannesSommer opened this issue Jul 13, 2017 · 3 comments
Open

How to really use rosconsole_bridge? #12

HannesSommer opened this issue Jul 13, 2017 · 3 comments

Comments

@HannesSommer
Copy link

The requirement to use REGISTER_ROSCONSOLE_BRIDGE (e.g. https://github.com/ros/rosconsole_bridge/blob/indigo-devel/test/cleanup.cpp#L16) is not mentioned in http://wiki.ros.org/rosconsole_bridge, which states instead :

If you have a console_bridge application or library, you can have its output be written to /rosout by simply having your package depend on rosconsole_bridge.

However it doesn't work without REGISTER_ROSCONSOLE_BRIDGE; somewhere to actually cause the redirection.

Assuming that this is intended, a) it should be mentioned in the documentation, and b) there seems to be a design flaw not providing a clean solution for the following situation (among others):

Lib A depends on ROS and lib B, which uses console_bridge. Then lib A should ideally already activate rosconsole_bridge for lib B to nicely work as ROS-lib and therefore have somewhere REGISTER_ROSCONSOLE_BRIDGE;. However, in an executable that depends on A and an A' doing the same thing would end up redirecting the console twice through A and A'. This works but yields unclean shutdown because it is apparently not supported in https://github.com/ros/console_bridge/blob/master/src/console.cpp#L94 since there is only one previous_output_handler_.

All problems could (IMO) be solved simply by using REGISTER_ROSCONSOLE_BRIDGE within the rosconsole_bridge library itself and not in its clients. This should make sure that the redirection is only happening once per executable AND get rid of this requirement for client packages and make the simplicity promised in the wiki come true.
What is wrong with this approach?

@dirk-thomas
Copy link
Member

it should be mentioned in the documentation

Absolutely! Maybe you can add a note to the wiki page. Please link the diff here so future readers can find it if you do so.

All problems could (IMO) be solved simply by using REGISTER_ROSCONSOLE_BRIDGE within the rosconsole_bridge library itself and not in its clients. This should make sure that the redirection is only happening once per executable AND get rid of this requirement for client packages and make the simplicity promised in the wiki come true.

While that would be convenient it would also break existing code which does use the macro, e.g.:

So it needs to be ensured that multiple uses of the macro don't break the behavior if the macro is going to be used automatically within rosconsole_bridge.

@HannesSommer
Copy link
Author

Thanks for the feedback!

With the PR #13 it becomes trivial to truly support activation by linking without breaking existing code (thanks to the PR not changing ABI and (specified) API): the rosconsole_bridge library must only call REGISTER_ROSCONSOLE_BRIDGE; or equivalent itself. This could still visibly affect systems that did not use the macro before because for those rosconsole_bridge might become activated.
As a next step the public version of this macro could be deprecated and gradually removed from the clients (I've got the code for both steps already).

However, there is an issue with this original dream that I didn't think of before: linking to a library without using any of its symbols is not reliable if linking is performed on an "as needed" basis (GNU linker: --as-needed; per default on Debians according to ros/catkin#680).

The missing piece would be to deactivate "as needed" for this library specifically. But that seems to be difficult with cmake : http://cmake.3232098.n2.nabble.com/Override-CMAKE-SHARED-LINKER-FLAGS-for-one-particular-library-td7593385.html. Any ideas here?

If that turns out to be too difficult or too expensive I see two options how to proceed:

  • Bury the dream and fix the documentation about how to use rosconsole_bridge
  • Link to rosconsole_bridge indirectly e.g. through rosconsole OR just merge its entire functionality into rosconsole, which might be the most intuitive and seamless options of all.

@mpflanzer
Copy link

Since I stumbled across the same problem I now updated the package documentation. http://wiki.ros.org/action/info/rosconsole_bridge?action=diff&rev2=4&rev1=3

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

No branches or pull requests

3 participants