implement optional queueing for rospy publications (#169)

[dirk-thomas]

Specifying the maximum queue size when creating a publisher (

ros_comm/clients/rospy/src/rospy/topics.py

Line 742 in 69fb651

def __init__(self, name, data_class, subscriber_listener=None, tcp_nodelay=False, latch=False, headers=None, queue_size=None):

) will make all `publish() calls being handled asynchronously. The functionality is implemented as an non-default option in order to not change the default behavior.

There are several side effects to this change:

• changing the internal behavior to asynchronous IO will expose error cases only after a repeated publish() call to the user code (since they don't happen immediately but asynchronously)
• this implementation will actually start dropping messages on the publisher side if the queue size is exceeded which is not the case for the current/default blocking API
• user land code needs to be updated in order to leverage the new functionality (e.g. in order to address issues like in rospy publisher hanging when subscriber drops connection (e.g. because of wifi) #169)

@tfoote @wjwwood Please review.

[tfoote]

+1

[wjwwood]

Is the expected behavior that you:

• publish
• an error occurs asynchronously
• the next publish raises
• the next publish succeeds
• an error occurs asynchronously
• the next publish raises

and so on?

Does this make more sense than having the error state persist until explicitly cleared? I don't think this is better, but I just wanted to see what you thought.

[dirk-thomas]

Yes, the raise will happen on the subsequent call and only once (not repeatedly). Since non of the existing code knows about this new wrapper adding something for clearing the error is not feasible and I would prefer it not to raise forever.

As far as I have seen the code which uses this wrapper will always close the connection after an exception since the connection is in an unusable state anyway.

[wjwwood]

+1

[tfoote]

As it stands this is the best for hydro with minimal impact. The other consideration is what to do going forward. We should consider making this the default in Indigo.

[mikeferguson]

+1

Tested this out on our robot, works exactly as I was hoping. @dirk-thomas @tfoote thanks to both of you for taking care this!

[dirk-thomas]

Which publishers have you changed to make it work in your scenario? Then we can make sure that they get fixed in the near term.

[mikeferguson]

@dirk-thomas For the most part my issues were in some of my higher level scripts that I was easily able to update. Not too much of the lower-level infrastructure that OSRF maintains uses rospy, although I know SMACH was one place where this change will make a huge difference.

[mikeferguson]

I guess tf is the other place where this frequently can be a problem, since for instance, you often have tf subscribers in RVIZ on a remote machine.