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

add stealth mode for topic_tools/relay #1155

Merged
merged 1 commit into from
Oct 20, 2017

Conversation

furushchev
Copy link
Contributor

This PR is for adding "stealth mode" to topic_tools/relay node.
This is almost the same as a normal relay but it relays only if topic specified by param ~monitor_topic is being subscribed by any other nodes except itself.

When input topic of relay and monitor topic are pointing to the same topic (this is by default), it looks like the relay node subscribes input topic only when other nodes also subscribe the input topic, which means subscribing without increment subscription count.

This feature is useful for publishing topic for visualization with lazy transport (c.f. https://github.com/ros/nodelet_core/blob/indigo-devel/nodelet_topic_tools/include/nodelet_topic_tools/nodelet_lazy.h ).
For example, lazy-transport-enabled nodes don't subscribe input topics unless other nodes subscribe the output topics of the nodes, but some nodes are expected to always subscribe topics (e.g. RViZ) and this breaks "laziness" of the nodes.
By putting relay node with stealth mode between RViZ and lazy node, this issue will be solved.

@mikepurvis
Copy link
Member

This seems reasonable to me. 👍

Copy link
Member

@dirk-thomas dirk-thomas left a comment

Choose a reason for hiding this comment

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

The new test doesn't seem to be registered anywhere and therefore does not seem to be run?

g_sub = new ros::Subscriber(g_node->subscribe(g_input_topic, 10, &in_cb, g_th));
}

void unsubscribe()
{
ROS_DEBUG("unsubscribe");
Copy link
Member

Choose a reason for hiding this comment

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

Should these debug messages be removed?

@furushchev
Copy link
Contributor Author

@mikepurvis Thanks for a warm comment! 👍

@dirk-thomas Thanks for review and nice catch! Registered new test and removed debug logs.

@dirk-thomas
Copy link
Member

Looks good to me: http://build.ros.org/job/Lpr__ros_comm__ubuntu_xenial_amd64/481/testReport/__main__/TestRelayStealth/

Thank you for the patch. Please add some documentation about this new feature to the wiki page.

@furushchev
Copy link
Contributor Author

@dirk-thomas Sorry for late, updated documentation http://wiki.ros.org/topic_tools/relay 👍

@dirk-thomas
Copy link
Member

Thank you for adding the docs. I added a note that this feature is new in Lunar (http://wiki.ros.org/action/diff/topic_tools/relay?action=diff&rev1=18&rev2=19).

@furushchev
Copy link
Contributor Author

@dirk-thomas Thank you too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants