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 queue_size for publishers #86

Merged
merged 1 commit into from
Mar 27, 2014

Conversation

jonbinney
Copy link
Contributor

rospy publishers hang if one of their subscribers goes out of wifi range. After the TCP buffer to the non-responsive node fills up, writing data to the publisher's socket hangs, causing it to to block and causing other nodes to not receive data. To fix this, the queue_size parameter was added to rospy publishers a while back. It's been bad practice to use rospy publishers without setting queue_size for a while, but this fact hasn't been widely advertised.

With the queue_size argument set to some number, after buffering queue_size messages, messages to unresponsive nodes are dropped. This is almost always the desired behavior. rosbridge doesn't provide the queue_size param, so rospy defaults to the old, blocking style. Starting in indigo this will cause rospy to print a warning: ros/ros_comm#372

This PR sets the default queue_size to 100, but lets it be overridden by adding a queue_size parameter to the json message. I haven't implemented the rosjs side for this, but the default behavior should be what most people want.

To test this, it is useful to have a publisher that sends a lot of data to fill up the queue quickly. Try this: https://gist.github.com/jonbinney/9752997
First, start rosbridge. Second, connect to it with the provided webpage (publishes strings on cmd_vel). Now start two instances of rostopic hz cmd_vel -w3 in two separate terminals. Hit ctrl-z to backround one of them, and after less than a minute the other will hang. After this PR, it won't hang.

@jihoonl
Copy link
Member

jihoonl commented Mar 25, 2014

@tlau
Copy link

tlau commented Mar 27, 2014

+1 for merging into hydro. This is making rosbridge unusable for us on a wifi-based robot.

jihoonl added a commit that referenced this pull request Mar 27, 2014
use queue_size for publishers
@jihoonl jihoonl merged commit 7e94663 into RobotWebTools:hydro-devel Mar 27, 2014
@jihoonl
Copy link
Member

jihoonl commented Mar 27, 2014

thanks.

@jihoonl
Copy link
Member

jihoonl commented Mar 27, 2014

I have created a related issue in roslibjs to support client side as well.

@jonbinney
Copy link
Contributor Author

Thanks - adding client client side support will be nice, but things should work quite well without it.

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

3 participants