API review

Proposer: Dirk Thomas

Timeline: Please submit feedback by June 27, 2012.

Reviewers:

  • Thibault Kruse
  • Felix Kaser
  • Jack O'Quin
  • Christian Dornhege
  • Isaac Saito

See the APIReviewProcess for guidelines on doing an API review.

API Overview

The API to be reviewed here is the API exposed to ROS GUI plugin developers which covers the stacks qt_gui_core and rqt. It includes the interfaces for a Plugin, the PluginContext (in Python as well as C++) and the XML-description for a plugin.

The current implementation as well as several plugins can be found in the ROS GUI repository.

The Plugin interface is designed to be as-small-as-possible. It only contains methods necessary for the life cycle, setting and configuration management.

Any functionality to interact with the framework is provided by the PluginContext. It contains methods to contribute widgets, request closing the plugin and get additional information about the plugin instance.

Python

A plugin can (but does not have to) inherit from qt_gui.plugin.Plugin. The following methods should be implemented:

  • __init__(self, context)

    • Called during construction of a plugin instance and gets passed the plugin context to interact with the framework (contributing widgets etc.). The constructor is used instead of an init-method to keep the interface small.

  • shutdown_plugin(self)

    • Called before the plugin instance is closed. A method is used instead of the destructor to guarantee execution of clean-up code.

  • save_settings(self, plugin_settings, instance_settings)

    • Called so that the plugin can persist its intrinsic configuration. The settings can be stored either in a per-plugin storage or in a per-instance storage. First is useful to store i.e. recently used filenames across all instance of the the same plugin. Second is useful to store the state of checkboxes etc. of the specific instance.

  • restore_settings(self, plugin_settings, instance_settings)

    • Called so that the plugin can restore its previously saved intrinsic configuration.

  • trigger_configuration(self)

    • If implemented the title bar of the plugins dock widgets shows an extra icon which triggers this method. It is intended to spawn a modal dialog in this method which offers access to configuration settings of the plugin.

The qt_gui.plugin_context.PluginContext provides the following methods:

  • add_widget(self, widget)

    • Contribute a widget to the user interface. The framework will nest it into a dock widget and provide several common icons in the title bar.

  • remove_widget(self, widget)

    • Remove a previously added widget.

  • close_plugin(self)

    • Request to close the plugin instance. The framework will notify the plugin using the previously mentioned shutdown_plugin-method.

  • serial_number(self)

    • Get the serial number of the plugin. For a specific type of plugin each instance gets a serial number (which is the first not used positive integer at construction time).

C++

A plugin must inherit from rqt__gui__cpp::Plugin (which inherits from qt_gui_cpp::Plugin. The code uses camel case formatting (instead of underscore separated lower case) to fit well with the style of the Qt API. The following methods differ from the Python API:

  • Constructor()

    • The constructor does not have any arguments since this is currently not supported by the used pluginlib.

  • void init(PluginContext&)

    • Called directly after the constructor to pass-in the plugin context.

  • bool hasConfiguration() const

    • When overridden and returning true it indicates that the method trigger_configuration has been reimplemented (in Python that can be realized by checking if the method has been implemented).

The qt_gui_cpp::PluginContext provides the same interface (except naming style) as the Python equivalent.

Plugin Manifest

For C++ plugins the pluginlib documentation describes the plugin descriptions file and how to export it in the package manifest.

For Python the semantic of the plugin description file has been adapted to fit the specific needs. An example can be found in the tutorial.

In addition to the library- and class-tags (which are necessary to load the plugin) the tag qtgui is introduced which contains additional meta information about the plugin. These data are used to display the plugin in a menu (using a label, icon and status tip). Optionally menu items can be grouped hierarchically where as each level is described by its own group-tag.

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.

  1. (Thibault) Python API should indicate expected function return types, and argument types/interfaces (e.g. widget and instance_settings). Does add_widget have a bool response or an exception if adding the widget failed for any reason / bug?

  2. (Thibault) A common logging API can be quite useful. A standard plugin can then be delivered that shows the logged messages. (See http://help.eclipse.org/indigo/topic/org.eclipse.platform.doc.isv/reference/api/org/eclipse/core/runtime/Plugin.html#getLog%28%29)

    • (Dirk) I would propose to not integrate logging functionality into ROS GUI. All ROS plugins can use the ROS-specific logging functionality provided by the client libraries. A plugin similar to rxconsole can then be used to show/filter all these messages. Non-ROS plugins can decide if they want to use logging library or stick with the Qt functions qDebug, qWarning, qCritical.

      • (Thibault) I accept qt logging functions as viable solution.

      • (Isaac) By the way what are Non-ROS plugins in this case? (I know I respond too late)

        • A Non-ROS plugin is a plugin for qt_gui_core without having any dependency to ROS (i.e. a weather pluging).
  3. (Thibault) It is not clear to me whether a plugin has to create a widget, and whether it may create more than one widget on init. It seems that the API relies on a one-to-one relationship between plugin instance and widget, if so, this should be made more explicit.

    • (Dirk) A plugin can call add_widget:

      • never: then you only see the running plugin in the menu but no widgets
      • once: which is the most common use case
      • multiple times: to provide separately movable parts for the plugin UI
      • not only during construction but also anytime later: i.e. to dynamically extend the UI

    • (Dirk) Commonly any add_* method should be callable multiple times (as it is for the Qt equivalent) otherwise I would expect the method to be named set_*. I have entended the API documentation (https://kforge.ros.org/visualization/ros_gui?p=ros_gui;a=commitdiff;h=3975176110c5313e0fd4fa5aef6a3b74afdfa5eb) explicitly mentioning that it can be called arbitrary times. Do you think that is sufficient and clarifies how to use it?

    • (Felix) In my opinion this could be made even clearer. I think with the above explanation I understood what the semantics of add_widget is, but only by reading the API doc I would not be sure if you want me to create one big widget (e.g. a container with a box layout that contains all the other UI elements) or if there is a internal layout in place where I can add my UI elements one by one.

    • (Dirk) I have updated the API doc (https://kforge.ros.org/visualization/ros_gui?p=ros_gui;a=commitdiff;h=1cbaa5fdb2a32576f0a5da699fd8a7e5fdad2ef9#patch2) and tutorial (http://www.ros.org/wiki/rqt/Tutorials/Writing%20a%20Python%20Plugin?action=diff). Do you think that makes it clear? If not could you suggest a wording which is more helpful.

    • (Felix) Looks good.
    • (Thibault) The problem I see is that you can have one plugin with multiple instances, each with no or multiple widgets. If there is no widget, then each time the menu is invoked, you should get a new instance, but there is no way to delete that instance, is there? On the other hand, if one instance has multiple widgets, then the question is when is shutdown_plugin being called? When the user clicks the close button of the last open widget, or only when rosgui shuts down? Or do all widgets of one plugin instance get deleted simultaneously when one close button is clicked?

    • (Dirk) Each running plugin is listed in the menu Running Plugins and can be stopped from there.

    • (Dirk) Currently the plugin is shut down when the X-button on any widget of the plugin is clicked. I do see that this might not be very intuitive if a plugin provides multiple widgets. The problem here is that if the framework would only close/hide the specific widget the plugin needs to be able to handle that (using an event) which makes it more difficult for the plugin developers. Qt itself does it similar as you describe it: the last widget triggers a close of the app (if not configured otherwise). What are your thoughts - what would be the better approach?
  4. (Christian) add_widget should specify who takes possession of the widget, i.e. does a widget created with new have to be deleted by the plugin in shutdown_plugin() or is this handled by the API. (probably only relevant for C++)
  5. (Thibault) It may be good to define Exception types that API methods should throw
    • (Dirk) Currently non of the public API raises exceptions. Which ones would you propose and in which situation should they be raised?

    • (Thibault) I am mostly thinking of init, to tell rosgui that the plugin instance did not startup normally.

    • (Dirk) Any kind of exception needs to be catched by the framework (which is true for any call from the framework to a plugin). In that case the plugin not is initialized completely (and any widgets already contributed are garbaged automatically). So I do not see a reason to introduce a custom exception as it does not provide any additional value.
      • (Thibault) Some developers might like to see a difference between an Exception that is shown to the user vs. an exception that happens due to a bug. E.g. the following two line could have very different result in the UI: 
        • x = a/b # causes division by zero sometimes
raise RosGuiException('You need to start Gazebo first to use this plugin')
          It is then up to rosgui of how to deal with the different exceptions, but at least there would be a difference between a friendly exception or a mean one. E.g. for the friendly ones, rosgui can collect all of them during startup and display them in a single dialog, rather than opening one per plugin that fails.
      • (Dirk) I would argue that ROS GUI can handle exceptions reasonably without the need for a custom exception type. I don't think that a custom handling is needed. I would imagine the following:

        • If a plugin raises an exception ROS GUI shows a modal dialog with the exception string and offers the user to see the details e.g. a stack trace. Even if the exception would be of type RosGuiException the user should be informed of it the same way. On start-up, when multiple plugins are started automatically, ROS GUI can collect all exceptions and show them in one dialog.

  6. (Thibault) Use case: TF plugin (similar to rosrun tf view_frames) shows tf frames, I want to click on one tf frame and see to what other nodes the broadcaster is connected as a ROS node, in the rqt_graph plugin. This requires one plugin to cause another plugin to be started, and to display specific information. However it may not be useful to spawn a new widget every single time for this. So rqt_graph plugin may offer a service to display information about a node.
    • (Dirk) I would propose to use the approach described in the next comment.
  7. (Thibault) Use case "robot dashboard": Similar to pr2_dashboard, but allowing plugins to contribute to "health status" indicators. So that when a button turns red, the user may click it, and a corresponding widget appears showing some problem area. This also requires plugin communication and widget spawning.
    • (Dirk) Supporting plugin-to-plugin communication would make ROS GUI much more complex and it is not obvious how it should handle the case that each plugin might be instantiated multiple times. Therefore I would suggest keeping it as simple as possible and not provide that infrastructure.
      • (Dirk) Yes, if the serial number of the plugin is greater than the plugin knows that other (and even how many other) instances are currently open.
      • (Thibault) Both Eclipse as well as portal server frameworks have the same problems, and offer solutions. I do not feel well about using ROS topics/services for plugin communication, as I believe ROS topics should transport data that is robot related, not UI related, and it does not make sense to me that a roscore needs to be running for two plugins to communicate. But for now having no dedicated plugin communication is acceptable, I just wanted to have it mentioned in the API review, as this is a major topic to think about as rosgui evolves.
      • (Dirk) ROS GUI allows you to run each plugin in a separate process (to not crash the app if one plugin misbehaves). Since ROS provides methods to communicate between processes we should use them for that purpose instead of coming up with another ROS GUI specific solution.

    • (Dirk) Instead I would propose to use ROS topics/services to realize communication between plugins (as it is currently done for the dashboard). And a plugin can spawn additional widgets at any time calling add_widget on the context.

    • (Dirk) Even if it is supported to start a plugin in an already running GUI instance using the CLI option --command-start-plugin I would prefer if plugins do not start other plugins. I would suggest that one plugin advertizes a topic/service and another plugin will use that (and probably give a reasonable message when the topic/service is not available).

    • (Jack) Keeping the plugins independent of each other is a good simplification. If experience proves it necessary, more complex interfaces can be added later. Using ROS topics to communicate between plugins allows them to function independently.
  8. (Thibault) Widgets should be tab-able (allowed to run as tabs), and the API should reflect state changes to be visible only in the small tab area if the widget area is hidden. Another useful feature in similar projects is to allow widgets to be minimizeable (to icon size) and back. API should then reflect whether plugin area is visible or not.
    • (Dirk) Tab-able is definitely a desired feature. But implementing it in a stable fashion has not yet been achieved. This feature can be added in the future without changing the API since it does not need to expose any information to the plugin.

    • (Dirk) Allowing to minimize a plugin is also a useful enhancement. But this does also not require an API change since the notification of the plugin can be performed through the standard mechanisms of Qt (listing to the showEvent/hideEvent of the added widget in order to be notified when it changes visibiliy status).

  9. (Thibault) ROS node properties of ros_gui need to be clarified, to avoid e.g. duplicate node init, or duplicate tf listeners.

    • (Dirk) ROS plugins should NOT call init_node at all since the framework already does that. I have added this important information to the API doc (https://kforge.ros.org/visualization/ros_gui?p=ros_gui;a=commitdiff;h=f8dc402afb659c17dbe94c8e4f9c042bbb5d923f).

    • (Dirk) What do you mean with duplicate tf listeners? I would be guessing that you refer to the fact that if i.e. multiple Python plugins subscribe to the tf topic the data is transfered multiple times?
      • (Thibault) I mean: 
        • >>> listener = tf.TransformListener()
>>> listener2 = tf.TransformListener()
>>> Exception in thread Thread-7:
ServiceException: service [/tf_turtle/tf_frames] already registered
    • (Dirk) I would say that this problem is a design problem in tf. How would you deal with that in ROS GUI (except that each plugin maintains its own ROS node)?

      • (Jack) Since you don't want each plugin to maintain a separate ROS node, it appears that ROS GUI should provide a TransformListener() for them to use.

        • (Thibault) The better solution would IMO be to have plugin communication and plugin dependencies, such that all plugins who deal with ros share such resources. ros_gui does not need to provide a communication or dependency framework for that, it can also just stand aside and let a python module that acts as a library for ros plugins do that with global variables.
        • (Dirk) If plugin communication should be possible it must be provided by ROS GUI since the plugins can not assume that they even run in the same Python runtime. But since this feature would make ROS GUI much more complex I suggest to go without it (for now) and see how ROS GUI is used by the community as it is. If there is a strong need for that in the future we can extend the framework anytime without breaking anything.
      • (Dirk) ROS GUI must not have any dependencies like that - it would be a very ugly hack. This must be handled in plugin-space. I don't know tf very well but if that is the case the API must be changed to support that use case.

      • (Jack) I don't like it either. You can try getting `tf` changes into Groovy. There is a new tf2 implementation in the development pipeline; I don't know whether it has the same problem with multiple listeners. But, requiring a new interface to a fundamental component like tf is going to significantly delay ROS GUI deployment.

      • (Dirk) The issue does not influence the release of ROS GUI. First, there is no plugin listening for tf messages yet. Second, you could still use such a plugin but could not open a second instance. So if tf is not capable to support multiple listeners yet it just reduces the usability of that specific plugin.

      • (Jack) If I understood Thibault's example correctly, there can only be one plugin using tf at a time in the whole ROS GUI process. Two different tf plugins could not cooexist. That seems like a big problem. How do the existing plugins avoid using tf? Is this only a problem in Python, or are C++ plugins broken as well?

    • (Dirk) Based on the feedback on that topic I would conclude that it is still desired that all ROS GUI plugins run in the same node context. Based on that a plugin using tf has to deal with this issue and this feedback should be passed to the tf SIG (I will do that) to improve the API in the future.

      • (Jack) I implied agreement with that concept, but I don't much care one way or the other. My impression is that there are a few things that tend to break in ROS when there are multiple nodes in a single address space. So, a single ros_init() seems like a reasonable design point, but I don't know the exact trade-offs.

      • (Dirk) I do agree that ROS currently falls short in several of these cases. But we should not try to work around these issues in ROS GUI.

    • (Dirk) I would propose to keep the default behavior of ROS GUI to run all plugins in the same process sharing one node (per programming language) (since ROS currently does not support multiple nodes per process). If there is need to run them in separate nodes the user must use the CLI option --multi-process. In the future we might add options/configurations to only run specific plugins in separate processes or enable the ROS client libraries to support multiple nodes per process.

Conclusion

  • The API documentation has been updated in various locations (see 1., 3., 4. and 9.)
  • ROS GUI does not provide logging funtionality but relies on either the Qt or the ROS logging functions (following the KISS principle) (see 2.)
  • ROS GUI does not provide inter-plugin communication but relies on the ROS communication primitives (following the KISS principle) (see 7.)

  • Limitations of the client libraries when running all plugin in the same node can not be solved by ROS GUI but can be work around with the multi-process mode (9.)

  • The following additional features would be valueable and should be implemented in the future (but do not require an API change):

    • /!\ Collect plugin exceptions and show them to the user in a single dialog (instead of only on the console) (see 5.)

    • /!\ Allow dock-widgets to be tab-able (8.)

    • /!\ Allow dock-widgets to be minimized (8.)

Wiki: rqt/Reviews/2012-06-20_API_Review (last edited 2012-10-25 19:35:36 by DirkThomas)

Except where otherwise noted, the ROS wiki is licensed under the
Creative Commons Attribution 3.0