API review

Proposer: Tully Foote

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.

Josh

• std::vector< std::string > getFrameStrings ()

  • This is very inefficient (performs a full copy on the vector). I would prefer:

    • void getFrameStrings( std::vector< std::string >& frames );

    • or:

    • void getFrameStrings( std::vector< std::string >* frames );

  • Or, if you already have this exact list internally and are just returning it:

    • const std::vector< std::string >& getFrameStrings();

  • kwc: I would vote for the void getFrameStrings() versions so that client code isn't holding a reference to a mutable data structure

Eric

• In TransformListener, there is a TransformVector but no TransformPoint. Even if we don't enforce this with the type system, I think having a method to transform both vectors and points is important.

• "transform navively" should be either naive or native. I think native.
• Why aren't we using ros::time? The package already depends on roscpp, rosthread, rosexception.

• Does using the time and frame id already present in the Stamped output make sense for TransformStamped? It gets rid of two arguments, but at the cost of making the data_out both an input and output argument (the semantics would be that you fill in the frame and time, and then tf fills in the data for you)

• Which frame does the stamp on the output from lookupTransform have? Is it the source or the destination frame? The stamp on a transform isn't really useful without both of them.

Stu

• Where's the equivalent of Pose3D?

Jeremy

• How about renaming TransformSender to TransformBroadcaster? This seems like a better parallel than sender/listener.

• I'm still uncomfortable with the idea of Transforming a "vector" and having it get both translated and rotated as a "point". I'm equally uncomfortable with using a Transform to represent a Pose for much the same reason. I realize this is the same bag of worms we never resolved at the other meeting. I don't expect it to necessarily be reopened, but I just want to be on the record as not approving of it.

• In fitting with Eric's comment about TransformVector vs. TransformPoint, if tf::Transform is going to be a primitive type, used to store both transforms and poses, I feel we should also have a native implementation of TransformTransform (though maybe called TransformPose).

Meeting agenda

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

Conclusion

Package status change (also mark change on PackageStatusDict page)

• Add void getFrameStrings(std::vector<std::string>& frames )

• Rename to Broadcaster from Sender

• Document data types better

  • Bullet vs tf
  • write up semantic differences from above (point vs vector etc.)

• switch to ros::time. I didn't want to require linking against roscpp for the library, but the build system isn't good enough anyway to not do it anyway.

• Fix navively typo

• Create 5 data types quaternion, point, vector, pose, transform

  • transform will have a source

---