Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rewrite of rqt_console #186

Merged
merged 1 commit into from
Sep 29, 2013
Merged

rewrite of rqt_console #186

merged 1 commit into from
Sep 29, 2013

Conversation

dirk-thomas
Copy link
Contributor

A major rewrite of rqt_console mainly to overcome performance problems of the previous implementation.

The only operation which still takes a long time is when the view needs to add a large number of rows which has not been there before. This is completely related to sorting. Without sorting but using the model order the operations are much faster.

@dirk-thomas
Copy link
Contributor Author

@ablasdel @ahendrix @jonbinney @tkruse since you have all been involved on at least one of the above mentioned tickets:
I still have some minor things to do but the major improvements are completed and would be great if you can test/evaluate them. Can you please try this branch of rqt_console and provide feedback if it works for you or what needs further improvements?

@jonbinney
Copy link

Much improved, thanks dirk! I just tried spamming 1000 messages per second and adding/removing filters, and the interface remained responsive. Also loading thousands of saved messages is super quick. A few minor issues:

When I add the "severities" filter, "info" and "error" in the filter have a grey background. What does that mean? Clicking on them doesn't add or remove the grey background, instead it highlights them in orange (the default "selected" color ). ---- After typing this realized that the fields are just alternated gray and white, to separate them visually. But it confused me, and other people might also think that the 2 gray fields are selected.

It seems that i can only add one filter of each type (well, one of each type for exclude, and one of each type for include). Might be annoying for users who want to look for messages "containing foo" and also "containing bar".

Saving the messages only saves the messages currently being displayed. I can see why this will often be the desired behavior, but sometimes the user may want to save all messages so that they can load use different filters later, even if they are using some filters currently. Maybe an option in the "Save" dialog?

@dirk-thomas
Copy link
Contributor Author

I can see why the alternate colors might be difficult but I think they are still helpful especially when using the node/topic filter. Else it becomes very difficult to visually separate the items. Update: after talking with some people I still think that the alternate colors are a bigger improvement than they cause confusion.

Allowing only one filter per type was a design decision to keep it simple. E.g. you could always toggle the regexp and search for either foo or bar. I am not sure if we should allow multiple message/location filters. What do other people think about this? Update: multiple message/location filters can be added now again.

In order to save all messages you can always disable all filters and then export the list. I am not sure if we should introduce more buttons for this since if you don't filter just adds stuff to the UI which becomes more difficult to understand and uses more space. Update: if there is no strong feedback by multiple people I will keep it simple for now.

Thanks for the feedback - after some more feedback I will think about the trade offs.

@ablasdel
Copy link
Contributor

First off this is great! Thanks for looking into this and fixing up my shoddy code.

At the moment multiple message filters are allowed so people don't have to know regex to exclude/include multiple strings. I would fall on the side of leaving this as is to make it usable by a wider audiance.

Now that dirk has fixed the speed issues I think adding an extra save button when you can just disable the filters is a bit overkill. We have way too many buttons on the top as it is. If we absolutely have to add something I would suggest some way to enable/disable all filters at the same time.

@dirk-thomas
Copy link
Contributor Author

Currently every filter can only be added once. As Aaron mentioned the latest release allowed to add multiple message filters. I broke that in 25cb985#L6L244 and will fix it soonish.

@dirk-thomas
Copy link
Contributor Author

As long a no column the view uses the model order which improves the performance even when loading data or changing filters in a way that a lot of rows reappear. Currently it is not possible to switch back to model order after selecting a column. I think we need a button or special mouse click for that.

@tkruse
Copy link
Contributor

tkruse commented Sep 27, 2013

I see no difference to my ordering problem, but that might be because
overlaying does not work. Is this supposed to work with a catkin workspace
overlaying a system install of hydro, and just the rqt_console package in
the catkin workspace source folder?

On Fri, Sep 27, 2013 at 3:20 AM, Dirk Thomas notifications@github.comwrote:

As long a no column the view uses the model order which improves the
performance even when loading data or changing filters in a way that a lot
of rows reappear. Currently it is not possible to switch back to model
order after selecting a column. I think we need a button or special mouse
click for that.


Reply to this email directly or view it on GitHubhttps://github.com//pull/186#issuecomment-25216410
.

@tkruse
Copy link
Contributor

tkruse commented Sep 27, 2013

Ah, sorry, I was not working on the special branch. OK, with this branch,
the sorting problem did not occur for me, but also the timestamps were so
fine-grained that no two log messages had the same timestamp. So I cannot
tell whether the 2nd level sorting works as supposed, but it seems with
fine grained timestamps that is much less of a concern in any case.

On Fri, Sep 27, 2013 at 6:58 PM, Thibault Kruse tibokruse@googlemail.comwrote:

I see no difference to my ordering problem, but that might be because
overlaying does not work. Is this supposed to work with a catkin workspace
overlaying a system install of hydro, and just the rqt_console package in
the catkin workspace source folder?

On Fri, Sep 27, 2013 at 3:20 AM, Dirk Thomas notifications@github.comwrote:

As long a no column the view uses the model order which improves the
performance even when loading data or changing filters in a way that a lot
of rows reappear. Currently it is not possible to switch back to model
order after selecting a column. I think we need a button or special mouse
click for that.


Reply to this email directly or view it on GitHubhttps://github.com//pull/186#issuecomment-25216410
.

@dirk-thomas
Copy link
Contributor Author

The ordering problem is not only addressed for the timestamp but for all columns (https://github.com/ros-visualization/rqt_common_plugins/pull/186/files#L21R95). So you can either reproduce it with timestamp which are identifical (even with nanosecond accuracy) or with any other equal values in any column.

@dirk-thomas
Copy link
Contributor Author

As long as no custom sorting is triggered by clicking on the column headers the performance should be awesome 😄 By clicking on the column header of the first column you can switch back to model order. The vertical header has a tooltip describing the behavior.

@dirk-thomas
Copy link
Contributor Author

I finally got the last polishing commits in, fixed some more tiny issues and squashed the commits. If there is no further feedback I will go ahead, merge and release it soonish.

@dirk-thomas
Copy link
Contributor Author

@wjwwood Please "review" - due to size of the diff I actually don't expect a full code review though 😉

@ablasdel
Copy link
Contributor

Just remembered that Isaac reused the console subscriber in another plugin. If we rename it we will have to make sure to rename it there too.

I looked around and I can't find where he used it. Perhaps he stopped doing so. Ignorethis for now.

@wjwwood
Copy link
Member

wjwwood commented Sep 27, 2013

Builds and runs well on the mac, as far as I can see. Loaded a test file @dirk-thomas gave me, seems to work.

screen shot 2013-09-27 at 3 04 46 pm

@dirk-thomas
Copy link
Contributor Author

Very good point @ablasdel. I update rqt_robot_dashboard to work with the updated version of rqt_console: see pull request ros-visualization/rqt_robot_plugins#54.

dirk-thomas added a commit that referenced this pull request Sep 29, 2013
@dirk-thomas dirk-thomas merged commit 12dcde2 into groovy-devel Sep 29, 2013
@dirk-thomas dirk-thomas deleted the rqt_console_rewrite branch September 29, 2013 18:59
dirk-thomas added a commit to ros-visualization/rqt_robot_plugins that referenced this pull request Sep 29, 2013
severin-lemaignan pushed a commit to severin-lemaignan/robotpkg that referenced this pull request Aug 18, 2014
Changes since 0.2.16:

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Changelog for package rqt_reconfigure
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0.3.8 (2014-07-15)
------------------

0.3.7 (2014-07-11)
------------------
* fix slider bar, add context menus for common operations (`#251
<https://github.com/ros-visualization/rqt_common_plugins/issues/251>`_)
* fix bug in float range calculations (`#241
<https://github.com/ros-visualization/rqt_common_plugins/issues/241>`_)
* remove experimental suffix from rqt_reconfigure (`#256
<https://github.com/ros-visualization/rqt_common_plugins/issues/256>`_)
* export architecture_independent flag in package.xml (`#254
<https://github.com/ros-visualization/rqt_common_plugins/issues/254>`_)

0.3.6 (2014-06-02)
------------------
* remove unnecessary margins to improve usability on small screens (`#228
<https://github.com/ros-visualization/rqt_common_plugins/issues/228>`_)

0.3.5 (2014-05-07)
------------------
* numerous improvements and bug fixes (`#209
<https://github.com/ros-visualization/rqt_common_plugins/pull/209>`_, `#210
<https://github.com/ros-visualization/rqt_common_plugins/pull/210>`_)
* add option to open list of names from command line (`#214
<https://github.com/ros-visualization/rqt_common_plugins/pull/214>`_)

0.3.4 (2014-01-28)
------------------

0.3.3 (2014-01-08)
------------------
* add groups for rqt plugins, renamed some plugins (`#167
<https://github.com/ros-visualization/rqt_common_plugins/issues/167>`_)
* mark rqt_launch and rqt_reconfigure as experimental (`#167
<https://github.com/ros-visualization/rqt_common_plugins/issues/167>`_)

0.3.2 (2013-10-14)
------------------

0.3.1 (2013-10-09)
------------------

0.3.0 (2013-08-28)
------------------
* fix updating range limits (`#108
<https://github.com/ros-visualization/rqt_common_plugins/issues/108>`_)
* fix layout quirks (`#150
<https://github.com/ros-visualization/rqt_common_plugins/issues/150>`_)
* fix icon for closing a node (`#48
<https://github.com/ros-visualization/rqt_common_plugins/issues/48>`_)
* fix handling of enum parameters with strings

0.2.17 (2013-07-04)
-------------------
* Improvement; "GUI hangs for awhile or completely, when any one of nodes
doesn't return any value" (`#81
<https://github.com/ros-visualization/rqt_common_plugins/issues/81>`_)

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Changelog for package rqt_topic
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0.3.8 (2014-07-15)
------------------

0.3.7 (2014-07-11)
------------------
* export architecture_independent flag in package.xml (`#254
<https://github.com/ros-visualization/rqt_common_plugins/issues/254>`_)

0.3.6 (2014-06-02)
------------------

0.3.5 (2014-05-07)
------------------

0.3.4 (2014-01-28)
------------------

0.3.3 (2014-01-08)
------------------
* add groups for rqt plugins, renamed some plugins (`#167
<https://github.com/ros-visualization/rqt_common_plugins/issues/167>`_)
* catch and show exceptions `#198
<https://github.com/ros-visualization/rqt_common_plugins/issues/198>`_

0.3.2 (2013-10-14)
------------------

0.3.1 (2013-10-09)
------------------
* improve rqt_topic initialization time (`#62
<https://github.com/ros-visualization/rqt_common_plugins/issues/62>`_)
* modified toggling topics to use checkbox instead of context menu (`#75
<https://github.com/ros-visualization/rqt_common_plugins/issues/75>`_)

0.3.0 (2013-08-28)
------------------
* fix cleaning old data in rqt_topic (fix `#74
<https://github.com/ros-visualization/rqt_common_plugins/issues/74>`_)

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Changelog for package rqt_top
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0.3.8 (2014-07-15)
------------------

0.3.7 (2014-07-11)
------------------
* export architecture_independent flag in package.xml (`#254
<https://github.com/ros-visualization/rqt_common_plugins/issues/254>`_)

0.3.6 (2014-06-02)
------------------

0.3.5 (2014-05-07)
------------------

0.3.4 (2014-01-28)
------------------
* fix sort order for numerical fields (`#205
<https://github.com/ros-visualization/rqt_common_plugins/issues/205>`_)

0.3.3 (2014-01-08)
------------------
* add groups for rqt plugins, renamed some plugins (`#167
<https://github.com/ros-visualization/rqt_common_plugins/issues/167>`_)
* fix an error caused by SIGKILLing nodes

0.3.2 (2013-10-14)
------------------

0.3.1 (2013-10-09)
------------------

0.3.0 (2013-08-28)
------------------
* remove copy of psutil module and implement missing function (`#105
<https://github.com/ros-visualization/rqt_common_plugins/issues/105>`_)

0.2.17 (2013-07-06)
-------------------
* Embeds python-psutil in the package in order to be enabled on Ubuntu Precise
* first release of this package into hydro

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Changelog for package rqt_image_view
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0.3.8 (2014-07-15)
------------------

0.3.7 (2014-07-11)
------------------

0.3.6 (2014-06-02)
------------------

0.3.5 (2014-05-07)
------------------
* list image transport topics if parent image topic is not available (`#215
<https://github.com/ros-visualization/rqt_common_plugins/issues/215>`_)

0.3.4 (2014-01-28)
------------------

0.3.3 (2014-01-08)
------------------
* add groups for rqt plugins, renamed some plugins (`#167
<https://github.com/ros-visualization/rqt_common_plugins/issues/167>`_)
* properly handle aligned images
* wrap cv calls in try-catch-block (`#201
<https://github.com/ros-visualization/rqt_common_plugins/issues/201>`_)

0.3.2 (2013-10-14)
------------------

0.3.1 (2013-10-09)
------------------
* fix event handling for rqt_image_view enabling to run multiple instances
simultaneously (`#66
<https://github.com/ros-visualization/rqt_common_plugins/issues/66>`_)
* add rqt_image_view to global bin (`#168
<https://github.com/ros-visualization/rqt_common_plugins/issues/168>`_)

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Changelog for package rqt_bag_plugins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0.3.8 (2014-07-15)
------------------
* fix missing installation of resource subfolder

0.3.7 (2014-07-11)
------------------
* add plotting plugin (`#239
<https://github.com/ros-visualization/rqt_common_plugins/issues/239>`_)
* fix rqt_bag to plot array members (`#253
<https://github.com/ros-visualization/rqt_common_plugins/issues/253>`_)
* export architecture_independent flag in package.xml (`#254
<https://github.com/ros-visualization/rqt_common_plugins/issues/254>`_)

0.3.6 (2014-06-02)
------------------

0.3.5 (2014-05-07)
------------------
* fix PIL/Pillow error (`#224
<https://github.com/ros-visualization/rqt_common_plugins/issues/224>`_)

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Changelog for package rqt_plot
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0.3.8 (2014-07-15)
------------------
* fix missing installation of Python subpackage

0.3.7 (2014-07-11)
------------------
* fix missing import (`#248
<https://github.com/ros-visualization/rqt_common_plugins/issues/248>`_)
* significant improvements and unification of different plot backends (`#239
<https://github.com/ros-visualization/rqt_common_plugins/issues/239>`_, `#231
<https://github.com/ros-visualization/rqt_common_plugins/issues/231>`_)
* make more things plottable including arrays and simple message types (`#246
<https://github.com/ros-visualization/rqt_common_plugins/issues/246>`_)
* make DataPlot a proxy for its plot widget, redraw after loading new data, add
clear_values (`#236
<https://github.com/ros-visualization/rqt_common_plugins/issues/236>`_)
* export architecture_independent flag in package.xml (`#254
<https://github.com/ros-visualization/rqt_common_plugins/issues/254>`_)

0.3.6 (2014-06-02)
------------------
* subscribe to any known topic, even if currently not available (`#233
<https://github.com/ros-visualization/rqt_common_plugins/pull/233>`_)

0.3.5 (2014-05-07)
------------------
* change minimum padding to enable viewing arbitrarily small values (`#223
<https://github.com/ros-visualization/rqt_common_plugins/pull/223>`_)
* redraw plot only on new data to reduce cpu load, especially with matplot
(`#219 <https://github.com/ros-visualization/rqt_common_plugins/issues/219>`_)

0.3.4 (2014-01-28)
------------------

0.3.3 (2014-01-08)
------------------
* add groups for rqt plugins, renamed some plugins (`#167
<https://github.com/ros-visualization/rqt_common_plugins/issues/167>`_)
* add checkbox to toggle automatic scrolling of plot with data
* add simple legend for pyqtgraph backend

0.3.2 (2013-10-14)
------------------

0.3.1 (2013-10-09)
------------------

0.3.0 (2013-08-28)
------------------
* fix waiting on unpublished topics (`#110
<https://github.com/ros-visualization/rqt_common_plugins/issues/110>`_)
* fix rendering of icons on OS X (`ros-visualization/rqt#83
<https://github.com/ros-visualization/rqt/issues/83>`_)

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Changelog for package rqt_console
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0.3.8 (2014-07-15)
------------------

0.3.7 (2014-07-11)
------------------
* export architecture_independent flag in package.xml (`#254
<https://github.com/ros-visualization/rqt_common_plugins/issues/254>`_)

0.3.6 (2014-06-02)
------------------

0.3.5 (2014-05-07)
------------------

0.3.4 (2014-01-28)
------------------

0.3.3 (2014-01-08)
------------------
* add groups for rqt plugins, renamed some plugins (`#167
<https://github.com/ros-visualization/rqt_common_plugins/issues/167>`_)
* use icons instead of text when available, refactor pause/resume button

0.3.2 (2013-10-14)
------------------

0.3.1 (2013-10-09)
------------------
* rewrite of rqt_console to drastically improve performance (`#186
<https://github.com/ros-visualization/rqt_common_plugins/pull/186>`_)

0.3.0 (2013-08-28)
------------------
* pause button no more saves state (`#125
<https://github.com/ros-visualization/rqt_common_plugins/issues/125>`_)
* persist message limit (`#138
<https://github.com/ros-visualization/rqt_common_plugins/issues/138>`_)
* add ability to set logger level (`#117
<https://github.com/ros-visualization/rqt_common_plugins/issues/117>`_)
* add tooltips to table cells (`#143
<https://github.com/ros-visualization/rqt_common_plugins/issues/143>`_)
* improve labels for filters (`#146
<https://github.com/ros-visualization/rqt_common_plugins/issues/146>`_)
* fix time column when loading data from file (`#160
<https://github.com/ros-visualization/rqt_common_plugins/issues/160>`_)
* fix applying message limit on change (`#133
<https://github.com/ros-visualization/rqt_common_plugins/issues/133>`_)
* fix clear button to remove all messages (`#141
<https://github.com/ros-visualization/rqt_common_plugins/issues/141>`_)
* fix sorting to use row index to decide order between equal values (except for
time column) (`#124
<https://github.com/ros-visualization/rqt_common_plugins/issues/124>`_)
* fix locking of message queue
* fix rendering of icons on OS X (`ros-visualization/rqt#83
<https://github.com/ros-visualization/rqt/issues/83>`_)

^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Changelog for package rqt_graph
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0.3.8 (2014-07-15)
------------------

0.3.7 (2014-07-11)
------------------
* fix compatibility with Groovy, use TopicStatistics only if available (`#252
<https://github.com/ros-visualization/rqt_common_plugins/issues/252>`_)
* export architecture_independent flag in package.xml (`#254
<https://github.com/ros-visualization/rqt_common_plugins/issues/254>`_)

0.3.6 (2014-06-02)
------------------

0.3.5 (2014-05-07)
------------------
* add displaying of topic/connection statistics along edges (`#214
<https://github.com/ros-visualization/rqt_common_plugins/pull/214>`_)
* using CATKIN_ENABLE_TESTING to optionally configure tests (`#220
<https://github.com/ros-visualization/rqt_common_plugins/pull/220>`_)

0.3.4 (2014-01-28)
------------------

0.3.3 (2014-01-08)
------------------
* add groups for rqt plugins, renamed some plugins (`#167
<https://github.com/ros-visualization/rqt_common_plugins/issues/167>`_)

0.3.2 (2013-10-14)
------------------

0.3.1 (2013-10-09)
------------------
* modified zooming method to work better on high-res trackpads like Macbook
Pros (`#187
<https://github.com/ros-visualization/rqt_common_plugins/pull/187>`_)

0.3.0 (2013-08-28)
------------------
* fix rendering of icons on OS X (`ros-visualization/rqt#83
<https://github.com/ros-visualization/rqt/issues/83>`_)

0.2.17 (2013-07-04)
-------------------
* Improve checkbox labels and tooltips wording.
dirk-thomas added a commit to ros-visualization/rqt_robot_dashboard that referenced this pull request Apr 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants