Added queue_size option as required in Indigo to prevent warning

[davetcoleman]

Fix for RethinkRobotics/baxter_examples#32

I'm not 100% sure if it should be value 10 or None, but None doesn't remove the warning message

@dirk-thomas why does not suppress the error even though it was the default in groovy and earlier? Do you know why was this change made in Indigo anyway when it makes ROS a bit more complicated?

Thanks!

[dirk-thomas]

None implies no queue / single threaded publishing (default value). Since this is bad default behavior (one subscriber can block all other subscribers from receiving messages) it needed to be changed. Therefore you get a warning to update your code to use asynchronous publishing using a queue with a defined size.

The default behavior was not changed in order to not potentially breaking existing code.

For more information please take a look at ros/ros_comm#346.

[davetcoleman]

Ah, I had only read the paragraph with the keyword queue_size, but what you said was in the previous paragraph on http://wiki.ros.org/rospy/Overview/Publishers%20and%20Subscribers

I edited the section title for clarity.

Thanks for the explanation, makes a lot of sense. I assume 10 is a good value?

[dirk-thomas]

Depends on how fast you publish and how long you want "old" values to be queued. 😉

[rethink-rlinsalata]

(more change-specific comment in next comment) First though - Thanks a bunch @davetcoleman for the PR's! Definitely appreciate the contributions, one more thing off the list for next release, which means room to squeeze one other thing in ;)

Minor favor before we merge - Could you please branch (/rebase) your commits off the latest development branch (instead of including the master commit history) and make the pull request against the development branch here? Would be greatly appreciated.
Also, did you smoke test any of the examples or test things still worked against the robot with your changes?

[rethink-rlinsalata]

We've been discussing whether the Publisher queue_size for joint commands should be just 1? The thinking being that in any "critical" commands where you always want to make sure to use the latest command available, you want to override any old msgs to be published if you have a newer one -- i.e. never use old commands if newer exist.

More background: Generation Robots told us that the joint action server / control performed much better when they changed the queue_size for the joint_states Subscriber to just 1 (and set tcp_nodelay). This prevented the control loops from ever using old joint_state data if there was a newer msg available (and ameliorated the batching problem). See #20 from v1.0.0 release.

---

Meanwhile, the "interaction", indicator, I/O commands could have a larger queue size (e.g. digital_io, etc).

I also don't know of any references for a "good value", or are there any recommended testing procedures to determine if a value works well? (We run most loops at 100Hz by default, and often test our control loops at around 500Hz too, though I don't know what most users run at...?)

[davetcoleman]

Will address after ICRA deadline...

[davetcoleman]

Good to be back!

We used Baxter a lot for some unrelated experiements using the changes in this PR and did not notice any problems. I don't know what the best values would be though - I leave that to you guys to decide. I guess you could just leave it as the previous default (which is not recommended by OSRF)...

Let me know what value it should be and I can rebase the PR, or feel free to do it yourself.

[rethink-kmaroney]

+1 thanks for this.

I vote for a queue_size of None for now in maintaining current behavior.

Also, do you mind rebasing and pulling against development instead of master as Rob mentioned above?

@rethink-imcmahon - After merged, can you & QA investigate implications of using an actual queue_size/asynch comm?

[rethink-kmaroney]

It seems passing None explicitly will still warn.
http://docs.ros.org/api/rospy/html/rospy.topics-pysrc.html#Publisher.__init__

This is in contradiction to what the actual warning says..
"SyntaxWarning: The publisher should be created with an explicit keyword argument 'queue_size'. Please see http://wiki.ros.org/rospy/Overview/Publishers%20and%20Subscribers for more information."

The 'queue_size' argument is explicitly provided. A more logical synchronous value/default should be chosen.

[dirk-thomas]

Since None is the default value of the keyword argument passing the same value but explicitly will not change anything. The message will still be printed to warn you that you are still using the legacy synchronous behavior.

[davetcoleman]

thanks for the involvement guys!

@dirk-thomas it would be nice if someone with a good understanding of the general implications of different values for queue_size would document that on the wiki to help everyone upgrading to indigo.

this is in my opinion a classic example of ROS becoming harder for the average user to understand and use. my specialty and interest is not in message passing

[dirk-thomas]

There is no magic formula to select the value. It all depends on publish frequency, system load and responsiveness of the subscriber. The description in the wiki is as good as it gets - I couldn't come up with more information.

[davetcoleman]

Moving discussion to ros/ros_comm#507

[davetcoleman]

I agree with @rethink-rlinsalata's comment on which values to use.

Side note, I documented how to choose a good queue_size: http://wiki.ros.org/rospy/Overview/Publishers%20and%20Subscribers#Choosing_a_good_queue_size

[rethink-imcmahon]

Thanks for the pull request @davetcoleman. Merged after some tweaks #40

[davetcoleman]

Thanks!