API review

Proposer: Dave Hershberger

Timeline: Please submit feedback by June 13, 2012.

Reviewers:

• Add your name here when you add comments below.

See the APIReviewProcess for guidelines on doing an API review.

API Overview

The API to be reviewed here is the API exposed to Display plugin developers, with a special emphasis on the Property class and the Display class, and their relevant subclasses. Doxygen-generated API docs are here. I am in the process of porting RViz to this new API, so looking deeper into the code you may find things that don't look consistent because I haven't done them yet.

There are a few major changes from Fuerte rviz here:

• Property objects are no longer mirrored by PropertyWidgetItem objects.

• In Fuerte rviz, a tree of PropertyWidgetItem objects was the content of the Qt "model" for Qt's model-view pattern, and there was a parallel tree of Property objects, with links between them. Keeping the two trees consistent and correct without infinite loops back and forth was difficult. In the new version, the Property objects are themselves the content of the Qt "model".

• Property objects now own the data values they describe.

• In Fuerte rviz, Property objects had boost-based pointers to getter and setter functions. That meant that just declaring a single property took 4 lines of code, for instance:
  • float getSize() { return size_; }
  • void setSize( float new_size );
  • float size_;

  • FloatPropertyPtr size_property_;

• In most cases the getter was a single line, and the setter would store the value and then often take some action.
• Since properties now own their data, there are generally only two lines in the declaration:

  • FloatProperty* size_property_;

  • void updateSize();
• The Property class has a Qt signal, changed(), which is emitted whenever its value changes. The updateSize() slot above would get connected to the changed() signal and thus be called to do whatever action the setter used to do. To get the data is now a little more code, because you don't have your "float size_" member directly,

  you have to say "size_property_->getFloat()".

• Sometimes there is no action to be taken or the same action serves for many property changes, so there is essentially just a single line to define a property.
• Display is now a subclass of Property.

• The new Property objects can have child Properties (instead of that functionality being in a CategoryProperty like in Fuerte). A Display, like GridDisplay, is something that appears in the PropertyTreeWidget with a name ("Grid"), a value (boolean checkbox indicating enabled/disabled), and child Properties (size, spacing, etc). Making Display a subclass of Property brings along all that functionality essentially for free.

• In Fuerte rviz, Display was not a subclass of CategoryProperty, it just had a pointer to one.

• Each Display and Property can save or load itself from a YAML object.
• Groovy rviz will use a YAML config file format. Many objects in rviz will be able to save and load and will recursively call their childrens' save() and load() functions. This eliminates the need to pass around "prefix" strings to everything which saves to config files, because the "prefix" state is maintained in the YAML objects.
• The new config file format should be more amenable to human and machine editing than the old one.
• I plan to write a config file format conversion program to ease the transition.

There are many more changes of course, please check out the docs.

Many rviz Display subclasses have been ported already, so you can see how the usage looks. A very simple example is RangeDisplay: range_display.h and range_display.cpp.

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.

Jon Bohren (jbo at jhu dot edu)

I see that a lot has changed in the Groovy VisualizationManager when compared to the Fuerte VisualizationManager (including the removal of the DisplayWrapper). Can you elaborate on the changes with respect to librviz use?

Conclusion

To be completed after review period.

Package status change (mark change in manifest)

• Action items that need to be taken.

• Major issues that need to be resolved