ros-visualization / rqt_graph

http://wiki.ros.org/rqt_graph
26 stars 27 forks source link

Multilayer namespace nesting, grouping for tf and image topics, hiding for tf and dynamic reconfigure #9

Closed EliteMasterEric closed 7 years ago

EliteMasterEric commented 7 years ago

rqt_graph has had a long-standing problem; in real use cases, graphs quickly become unwieldy and unreadable, with dynamic reconfigure's parameter topics everywhere, and don't even start with images. This large commit includes a number of feature improvements designed to combat this, listed below:

These changes are dependent on pull request ros-visualization/qt_gui_core#87.

This pull request resolves issues ros-visualization/rqt_graph#3 and ros-visualization/rqt_graph#4.

Three files are attached to this pull request; the launch file I used for testing, a before picture, and an after picture, displaying what rqt_graph looks like before and after the changes.

before after-2 Attachments.zip

dirk-thomas commented 7 years ago

Thank you for improving this.

The problem with the size of this PR is that it make the review process unnecessary difficult. Can you please either split the different unrelated features into separate PRs or at least into separate commits. That will make it more manageable to review the individual pieces.

Also please avoid any unrelated changes (like https://github.com/ros-visualization/rqt_graph/pull/9/files#diff-9d45525c1c11e3b0e12360b5e5ac47f9R67) since they only make the patch bigger and less readable. If you care about cleanup up existing code like this (which would be great) please do so in a separate PR.

EliteMasterEric commented 7 years ago

@dirk-thomas The splitting of this pull request into constituent parts may take a bit of time; for the moment, however, I will be closing this pull request and creating individual ones for different features, since, as you mention, that will be easier to maintain in the long run.

Some of the unrelated changes like minor formatting differences and small padding changes in RqtGraph.ui are due to the software used when creating the pull request; these will be minimized as much as possible in the new requests.

EDIT: These changes are now re-implemented and cleaned up in ros-visualization/rqt_graph#13