Changed the build type to ament_python and fixed package to run with ros2 run#8
Conversation
ivanpauno
left a comment
There was a problem hiding this comment.
LGTM with comment addressed and green CI
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: ahcorde <ahcorde@gmail.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
ivanpauno
left a comment
There was a problem hiding this comment.
LGTM!
With this will be able to only run ros2 run rqt_msg rqt_msg but not rqt_msg.
That was also the case before, but it isn't the case for other rqt tools like rqt_graph.
| maintainer='Dirk Thomas, Dan Lazewatsky, Michael Lautman', | ||
| maintainer_email='dthomas@osrfoundation.org', |
There was a problem hiding this comment.
Unrelated: Can we add the actual maintainers here?
I'm sure Dirk isn't active, no idea about the others.
There was a problem hiding this comment.
+1, but since we have to update the package.xml too, what about doing it in another PR? I think @mjcarroll is the maintainer now. I'm not sure.
There was a problem hiding this comment.
sure we can do it in another PR, it's unrelated to the purpose of this one
We should sort that out. It's somewhat confusing. I think |
I'm not sure I agree. While running
I'm on the fence about this change. First, is it at all technically feasible to have both |
With ament/ament_cmake#328 in, we could install a console script correctly in both /bin and /<package_name>/lib correctly if the package is I can imagine two alternatives for
The disadvantage of the first option is that installing a non binary file in lib/ is arguably wrong (though I'm not sure if it's actually wrong). The disadvantage of option two is that "packageA" is not supposed to install packages in "lib/packageB", whereas anybody can install executables in "bin/". So looking for executables in |
|
Looking at rqt_graph, it looks like we are doing the first of your two options; installing the script into |
Sounds good to me. |
Actually, in that case we will need to run Maybe there's another way to detect if a file is a python script without relying on the file extension (?). |
I will point out that you can do that with Regardless of the path we take, I'd say we defer that to another set of PRs. Most |
|
Alright, as per offline discussion, we'll move forward with these changes as they are now. We'll defer improvements to the Windows situation for later. |
|
I was thinking that another way to install an executable in both The hack would be something like this:
I'm not sure if we are open to re-releasing colcon-ros at this point though, but I can give it a try if we are. |
Fixed package to run with
ros2 run rqt_msg rqt_msg. Related to this issue ros-visualization/rqt_console#20Signed-off-by: ahcorde ahcorde@gmail.com