ros-visualization / rqt_robot_plugins

9 stars 44 forks source link

rqt_robot_dashboard - change inheritance to be from QMainWindow or at least be from QWidget #92

Closed aleksandaratanasov closed 9 years ago

aleksandaratanasov commented 9 years ago

I'm working on a dashboard for our SR2 platform here at the KIT IPR and I can't stop asking myself why on Earth is this the way it is.

First we have the Plugin, which inherits directly from QObject adding as little as saving, restoring and shutting itself down, which at this point is fine. However the next one is the Dashboard, which - note - basically mimics QMainWindow but adds a bunch of extra stuff (not enough to give an excuse for reimplementing QMainWindow) and inherits from Plugin.

My suggestion is to keep the extra things that are required for ROS in a separate or a set of modules and leave the rest to what is already there - the Qt classes. I find the extra amount of work one has to do in order to add basic functionality like fullscreen mode or displaying a message in a statusbar (QStatusBar can only be added to a QWidget but since Dashboard is not derived from the QWidget class one has to do various tricks and implement a whole set of otherwise existing features in order to do that) just suicidal. And with that I don't want to insult the people behind the (Rqt) Dashboard - the idea is great! The design decisions however are something that I'd like to discuss here. Especially with the lack of documentation on the Dashboard-related things it's just wasting time in order to get small things done.

Anyone willing to make a small discussion about this? I'm definitely not a pro in Qt so this as well be nonsense, in which case I'd like to get an explanation what lead to this design.

dirk-thomas commented 9 years ago

The dashboards as any other rqt plugins are not (only) standalone applications but are designed to be used together with other plugins within the same application.

The plugin class is not inheriting from QWidget because it is not a widget. One plugin can contribute multiple widgets.

The dashboard class is only a QWIdget and not a QMainWindow because it is not the only entity in the GUI application. Other plugins can be siblings to the dashboard and therefore neither of them is a main window.

You can read more about the concept (http://wiki.ros.org/fuerte/Planning/ROS%20GUI/2011-09-28) and the API (http://wiki.ros.org/rqt/Reviews/2012-06-20_API_Review) in the ROS wiki.

aleksandaratanasov commented 9 years ago

Thank you for the links. I'm not saying that I dislike the Plugin-idea in general. And yes, a Plugin != QWidget. I am talking here about the Dashboard in particular and nothing else. You've mentioned something very important:

The dashboards as any other rqt plugins are not (only) standalone applications but are designed to be used together with other plugins within the same application.

The problem I see right now is that you are treating the Dashboard as purely being a Plugin (a thing that you can embed in Rqt along with other Rqt plugins) but neglect the standalone part almost completely. Right now Dashboard is great if you view it as a part of Rqt (where you can add other Rqt plugins, RViz, rqt graph etc.). However as a standalone application (which is part of its nature as you've mentioned) it lacks even basic features (the fullscreen mode being one of those). Hence the need to either change its inheritance model or add a new type of Plugin that is suitable for standalone applications (but still preserves the basic Plugin functionality like being embeddable in the Rqt). This new Plugin type should be used for all Rqt standalone applications (if the developer wants those to be completely standalone).

dirk-thomas commented 9 years ago

Clearly a plugin can not derive from QMainWindow since it needs to be composable with other plugins.

Maybe you should describe what you actually want to achieve and there is a way to do that even when being a plugin.

E.g. setting a text for the status bar does not scale if you run the dashboard plugin together with other plugins - they would just overwrite each other. You could e.g. add a status bar to the plugin widget instead. That approach would play nicely with other plugins.

Regarding "full screen": there is currently no flag to put rqt itself in full screen mode. But I don't see why that would be a problem to add in both cases when multiple plugins are used as well as in standalone mode?

aleksandaratanasov commented 9 years ago

Clearly a plugin can not derive from QMainWindow since it needs to be composable with other plugins.

That is why I suggest changing the Dashboard (in particular).

Maybe you should describe what you actually want to achieve and there is a way to do that even when being a plugin.

I want to use the Dashboard as both a standalone (it will be displayed in fullscreen on a relatively small touchscreen mounted on the service robot I'm working on) as well as Rqt-embeddable (it also has to also be viewable on a big desktop PC monitor where touch is not available and mouse+keyboard are the only means of input and using Rqt with multiple plugins displayed alongside each other is an option).

But I don't see why that would be a problem to add in both cases when multiple plugins are used as well as in standalone mode?

Because QWindowState is available only for QWidget and one has to implement manually resizing mechanims.

You could e.g. add a status bar to the plugin widget instead. That approach would play nicely with other plugins.

That is what I'm doing.

As I said it's the standalone side of Dashboard the is suffering a lot and not it's Plugin nature when you want to embed it in Rqt.

dirk-thomas commented 9 years ago

I am sorry but I don't understand your points.

Every plugin has one or multiple widgets and can implement anything for them. Commonly when using dynamic layouts the size will automatically adapt. If you need custom resizing that is fine to and it can be implemented for each widget accordingly. (No need for the plugin to be anything else then a QObject?)

E.g. the create_dashboard can be maximized and scales automatically to use all the available space.