feat: Added QWidget

[2b-t]

The disadvantage of the current code is that the widget itself is directly implemented in the plugin. This way somebody can't embed the GUI into his/her own GUI easily (like in this tutorial here). Therefore I have

• Separated the Plugin and the QWidget (just like most of the other rqt packages do)
• Renamed the files in a similar fashion to the other packages with underscores instead of camelcased filenames.

I made a simple example of how this can be useful here (also see image below). Furthermore this somehow seems to solves the issue pointed out in #2.

[awesomebytes]

Hey, first, let me apologize for taking forever to look at this. I switched jobs in between other things as you made the PR and forgot to look at it for a long while.

@2b-t Thank you very much for doing this work. I would have liked to have it done as a QWidget, but I did not get there!

I have a few comments in this PR which are basically minor in regards of returns that are either unnecessary (and I had some of them in my code already!) or that I'd prefer to have an explicit return of None to help other possible contributors understand what is going on by being explicit (this is a personal preference, I'm open to discussion as I may be missing something).

I'm keen to merge this. If you have the time to attend my comments, that would be great.

There's one thing that would worry me a bit, which is that the plugin name has changed from RqtEmbedWindowWidget to EmbedWindowWidget (which makes total sense) and that could break people's code that points to the old name. I wonder if we can do a wrapper for the old plugin name that just calls the new one, so when merging this into main we won't be breaking anyone's code?
Maybe it's a lost cause and it would break people's rqt configs anyways.

Again, thanks a lot, and I apologize for not merging straight away.

[2b-t]

Hey @awesomebytes
Thanks for your reply. No worries, I completely understand!
Coming from C++ I am used to return; but indeed return None might be more expressive and using no return at all in the initializer might be more pythonic.
Regarding the re-naming: I only tried to keep the nomenclature consistent to other Python rqt plug-ins. I think it should be possible to wrap it or simply alias it to guarantee backwards compatibility. I will have a look at it tomorrow!

[2b-t]

@awesomebytes I just removed all unnecessary returns and made the others explicitly return None. Furthermore I fixed a small bug that I introduced when creating the separate widget in the beginning that would show

PluginHandlerDirect._shutdown_plugin() plugin "rqt_embed_window/EmbedWindow#1" raised an exception:
Traceback (most recent call last):
  File "/opt/ros/melodic/lib/python2.7/dist-packages/qt_gui/plugin_handler_direct.py", line 87, in _shutdown_plugin
    self._plugin.shutdown_plugin()
  File "/home/user/catkin_ws/src/rqt_embed_window/src/rqt_embed_window/embed_window.py", line 40, in shutdown_plugin
    self._widget.kill_process()
  File "/home/user/catkin_ws/src/rqt_embed_window/src/rqt_embed_window/embed_window_widget.py", line 146, in kill_process
    if process:
NameError: global name 'process' is not defined

in case no process was added to the widget and the widget was closed.

---

I also checked back on the naming of the widget and the file that I changed from camel case to underscores removing the leading Rqt. The reason why I renamed it was for it to be consistent with the other official rqt packages like Rqt Publisher or Rqt Topic.
In my opinion the changes to the naming are not strictly relevant to the end-user as they not the intended public API: One would use the plugin by either launching it from rosrun rqt_gui rqt_gui or by launching it directly with rosrun rqt_embed_window rqt_embed_window. Both of these remain unaffected by the change.

The changes are only relevant in case somebody would refer to RqtEmbedWindow.py explicitly either by

• executing rosrun rqt_embed_window RqtEmbedWindow.py which should result in an error anyhow or
• including the file into their project which would only make sense if they would want to inherit from it in order to derive their own plugin.

In case you are worried that somebody might have done latter we could add a file called RqtEmbedWindow.py that imports embed_window.py with from .embed_window import EmbedWindow as RqtEmbedWindow (or alternatively imports it and defines an alias for the class RqtEmbedWindow = EmbedWindow) as the two plugins still share the exact same interface. If one would have touched the internals of the class though, their code might break in any case due to the introduction of the widget.

Taking these things into consideration I think this is a very unlikely scenario and I would still keep the changes in naming in order to be consistent with the other rqt packages.

[2b-t]

Hey @awesomebytes
Have you any updates on this?

[2b-t]

Hi @awesomebytes
Been a while now. Could we finally merge this?
I would also be interested in porting this to ROS 2 Humble and making another pull request for this at some point...