This also fixes a bug in extended_combo_box.py that prevented the widget from functioning.

The Python module and widget it provides works as-is. The modified line is only relevant when testing the module. It is great that this patch fixes this.

There is currently no filter on what topics are displayed; do we want to add one or more filters? A regex filter for the topic name? A filter on what topic types are allowed?

I guess the best indicator for this is what existing plugins are using. If they don't use a filter it should be ok to start without supporting one. If existing plugins do provide filtering options it would be good to enumerate them.

There is an issue with how PyQt is designed; it only allows modification of GUI elements from the main thread. I was initially going to implement a rospy.Timer to automatically update the list, but updating the list from the timer thread crashes the program. The current solution requires the application implementing the widget to run TopicComboBox.update_list() on some regular basis (such as when a refresh button is clicked) from the main GUI thread. If there is a better solution to this problem that puts less work on the implementors, please tell me.

This is not a "problem" in PyQt - this is the way the Qt event loop is implemented. The same restriction also applies to C++ code using Qt.

In general only the event loop thread can modify UI elements. But it is possible to interact with models from any threads - they internally will make sure to trigger UI update from the event loop in case it needs to be re-rendered. Maybe this is related to the fact that the patch currently replaces the model rather then updating it. Can you try doing that instead? I would also be worried that the current selection might not be kept when the model is replaced (rather then some items are added / removed from the existing model).

TopicComboBox and ActionComboBox currently use a hardcoded get_topic_list() method to get the list of topics; when ros/ros_comm#1154 is merged and rostopic has a public method for this, it should be removed.

Please see the pending comments on the ros_comm ticket which need to be resolved before it can be merged.

I've been looking into fixes for my changes, and found that I was implementing loops incorrectly; I should have been using QTimer() rather than ROS's timers, and updating the string list for the model instead of replacing the model.

I am going to wait until after the ross_comm ticket is done to create a final commit, however.

It has been a LONG time since I looked at this commit. Making it ready to merge relied on another pull request of mine, and that was delayed multiple times by PEP8 issues, and the fact I can't spend 100% of my time working on ROS PRs.

I removed the code that basically just copy-pasted the method I added in the other pull request, then fixed up a bunch of bugs because I'm a dummy and didn't even bother testing. Guess I thought "Oh, it's not going to get merged yet anyway, it has to wait for that other pull request" was a valid excuse, even though it just means more work for me now.

Anyway, this pull request is ACTUALLY done and tested; if anyone is looking at these changes, make sure you have the newest commit to ros_comm (which adds the required rostopic.get_topic_list() method).

@dirk-thomas What is needed to resolve this pull request? These changes are currently tested with kinetic and untested with Melodic. If you want, you can create a melodic-devel branch and I can merge my changes to it.