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.
- 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.
- 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.
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
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
Package status change mark change manifest)
Action items that need to be taken.
Major issues that need to be resolved