API review

Proposer: Josh Faust

Present at review:

• List reviewers

Question / concerns / comments

Enter your thoughts on the API and any questions / concerns you have here. Please sign your name. Anything you want to address in the API review should be marked down here before the start of the meeting.

Tully

• The doxygen for subscribe would be great to "explain" the difference between the forms of each call ala "Subscribe to a topic, w/ Shared pointer method" and there would be somewhere I could find documentation on recommended use cases for the different methods. For I can't differentiate between the versions by reading the prototype.
• Publisher::shutdown is public? is there a reason to have a Publisher which has been shutdown but not destructed?
• Same thing with Subscriber.shutdown()

• Publisher/Subscriber Should the default constructor be hidden/suppressed if only copies and NodeHandle::subscribe should create?

• ditto both above ServiceServer and ServiceClient

• hide ServiceClient.callimpl "for Internal use"?

• why both ServiceClient::isValid() and ServiceClient::valid() methods? same doxygen comments

Meeting agenda

To be filled out by proposer based on comments gathered during API review period

Conclusion

Package status change mark change manifest)

• Action items that need to be taken.

• Major issues that need to be resolved

• /!\ Add something to @brief description to differentiate between methods of the same name

• /!\ Add a note that Publisher::shutdown, Subscriber::shutdown, and NodeHandle::shutdown usually don't need to be called.

• /!\ Either hide ServiceClient::callimpl() or rename it call()

• /!\ Mark ServiceClient::valid() as deprecated

• /!\ Add examples of creating new handles with child scope

• /!\ How to handle library code that uses the NodeHandle API in an application that uses the Node API? Options:

  1. When a NodeHandle was created, if the global Node was created by the old API, then create a thread to call ros::spin()

     • ROS_WARN when this happens
  2. Make Node::ok() call ros::spinOnce()
     • Too slow; many apps don't call ok() that often
  3. Make the old application create a thread to call ros::spin()

Possible future work (post 0.5):

• /!\ Add support for periodic events via timers. Perhaps add it into the main event queue, perhaps make the user create a separate thread.