API review
Proposer: Stuart Glaser
There are several pages to look at:
http://wiki/bond - Overview
http://wiki/bondcpp - C++ implementation
http://wiki/bondpy - Python implementation
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
- I feel that this should be like actionlib with a ROS API defined and example implementations in python and c++
That's the plan. I haven't documented the internals for the API review, but I expect bonds to be implemented in other languages
- The timeouts are not exposed or documented. (What sort of expected frequency update/max delay for breaking etc)
Ok. How about exposing the timeouts through setter methods (to be called before start())?
- Are there nice(aka clean shutdown messages) ways to break a bond vs timeouts?
Yes. Calling break_bond() breaks the bond cleanly and quickly.
- How much overhead do they use? bandwidth, computation, threads
Insignificant computation. One thread in Python, but C++ uses a ROS timer. The bandwidth is a small heartbeat message at approx 1 Hz. It's been built to be extremely lightweight.
Eitan
- I think I'm OK with the API. I'm interested in resource management ideas and discussing the tradeoff between using something like a bond vs something like an action, but that's not really for this review.
Meeting agenda
To be filled out by proposer based on comments gathered during API review period
- Fix examples to match the API
- Jeremy doesn't like constructor to take a boolean as to whether you're the server or client. Prefers a client or server constructor.
- What if we just put in another unique id?
- Could have a startServer() startClient()
- The boolean means that the user could screw it up. Need to hide it or expose it completely.
- Solution: Unique id per bond instance to mark messages as your own.
- What if the timeouts don't match on the two sides? Could put the timeouts into the message itself.
- Include the timeouts in the bond status message, just for debugging purposes?
- Autonegotiating is too difficult, but with this information we can warn.
- Also gives someone else room to write the autonegotiation version.
- Naming the message's bool "alive" seems silly. Wim suggests "dying", "active". Because you shouldn't be sending messages if you are "Dead"
- Using a ROS timer could be an issue. If ROS isn't servicing its timers, it won't be publishing its messages. Maybe let the user specify a callback queue?
- Select a bond topic based on the id (inside a bond namespace) "/bond/id"
- Let ROS deal with collapsing huge numbers of topic.
- Discussion about remapping the bond topic.
- Take bond out of the global namespace?
- This would allow us to support bonds with IDs that are not unique.
- Tully: the bond id should be a globally-unique ROS name
- If we have a unique instance id, we can detect when too many things have the same bond id.
- Bond id is still unreadable. Difficult to debug
- Suggestion of a channel string and a bond id
BondID = string
BondID: string channel # Also the suggested topic to use for the bond string uuid # Only thing sent in the Bond Status message
- Error on empty channel
- Topic is helpful for finding the bond for a particular thing.
- The token that refers to a unique bond is the tuple (string topic, string uuid)
- topic cannot be blank
- Suggested to have "bond" in the topic name
Resulting Actions
Add an instance id to differentiate between the two sides
Change "alive" to "active" in the status message
Add setXxxTimeout calls
- Change bond token to be (topic, bond id)
Construct Bond objects using the topic
Use a topic which is topic/bond_id as the status topic
- () Place timeouts in the status message so we can match them up for debugging
- () Fix the examples to match the API
- Fix up comments:
- Bond now takes topic,id
Conclusion
Package status change mark change manifest)
Action items that need to be taken.
Major issues that need to be resolved