search

ros-visualization / rqt_graph

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

Add setup.cfg with script install directories #42

Closed ahcorde closed 4 years ago

ahcorde commented 4 years ago

Fixed package to run with ros2 run rqt_graph rqt_graph. Related to this issue https://github.com/ros-visualization/rqt_console/issues/20

ivanpauno commented 4 years ago

This doesn't look right. This is a CMake package so it doesn't use the added setup.cfg file at all.

It seems to me that it's an ament_python package:

https://github.com/ros-visualization/rqt_graph/blob/22952ef7604ed76f573cfae896c96c435203339a/package.xml#L29

Maybe you were checking master branch instead of crystal-devel?

dirk-thomas commented 4 years ago

Maybe you were checking master branch instead of crystal-devel?

I indeed did. Please ignore my comment.

ahcorde commented 4 years ago

Running CI up to rqt_graph

dirk-thomas commented 4 years ago

With this change you can't invoke rqt_graph anymore (without ros2 run) so this breaks existing use cases and documentation.

jacobperron commented 4 years ago

With this change you can't invoke rqt_graph anymore (without ros2 run) so this breaks existing use cases and documentation.

I hadn't considered this case. I think we should make a patch to re-enable this functionality before releasing (or revert this change).

jacobperron commented 4 years ago

Doing something similar to rqt_gui might work; adding an explicit executable to bin and installing it as a data file: https://github.com/ros-visualization/rqt/blob/b198efc2b1d4433f13442410e3a12ae5c4a1106b/rqt_gui/setup.py#L18

jacobperron commented 4 years ago

With this change you can't invoke rqt_graph anymore (without ros2 run) so this breaks existing use cases and documentation.

See https://github.com/ros-visualization/rqt_graph/pull/49 for a fix.