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.
- 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?
hide ServiceClient.callimpl "for Internal use"?
To be filled out by proposer based on comments gathered during API review period
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:
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
- Make Node::ok() call ros::spinOnce()
- Too slow; many apps don't call ok() that often
- 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.