add ros2 pkg executables

[dirk-thomas]

Fixes #21.

I ran the nosetests locally rather than triggering full CI builds.

Ready for review.

[mikaelarguedas]

Related to this: this function would be very useful to have available for launchfiles (rather than https://github.com/ros2/turtlebot2_demo/pull/46/files#diff-cbb1fb4c24441aafc45b0e5bd453b316R37).
Do we want launchfile to depend on this package or should this function moved to a package lower in the stack ?

[dirk-thomas]

The Python module api within each of these packages is supposed to provide a stable API to be consumed by other packages. So yes, other packages can depend on e.g. ros2pkg to use it.

[wjwwood]

I think it would be better if every package that has a launch file doesn't need to depend on the command line tool (doesn't make a whole lot of sense imo). So I would be in favor of a shared location for the logic and have both the launch files and command line tool use that.

That being said, we can ticket it and fix it later.

[mikaelarguedas]

I agree with @wjwwood, I'm fine with adressing it now or later. As soon as we have consensus of the state it'll be in the beta I'll update the launchfiles accordingly

[dirk-thomas]

All cli packages have an API module atm. A refactoring like this should be considered for after the beta 2.

[dirk-thomas]

Waiting for review.

[wjwwood]

Code changes lgtm, I haven't tried it. Maybe we should wait for one of the reporters to try it out.

[mikaelarguedas]

code looks good, I'm about to try it with some changes depending on it. Will approve once it's tested

[mikaelarguedas]

Maybe it's me not understanding what the initial request was but I was expecting a list of executable names not their absolute path. I think this should be either

• changes the behavior to give a list of basenames
• update the help message to states it's returning paths

I'd vote for the first one

[dirk-thomas]

Getting from the full path to the filename is trivial. Getting from the filename to the full path is not possible (without using the code from these Python modules). Therefore I would argue this verb must be able to return the full paths.

It would be ok to have an option to only return the filenames. Or showing the filename by default and having an option for showing the full path.

[mikaelarguedas]

Or showing the filename by default and having an option for showing the full path.

👍

[dirk-thomas]

Added in 10f3be9ced31ce7c2cd954ae862e897afdeb7dfa.

Note when printing the basenames of the executables across all packages they are sorted by package name and only as a secondary criteria by the basename. Not sure if that should be a global sort instead.

[mikaelarguedas]

I think it does make sense to print them sorted by package name (I mean the current sorting scheme). I'm wondering if we have a way to hint the package it's coming from to the user ( it will make it easier to know what the corresponding ros2 run invocation would be).

[mikaelarguedas]

Tested locally and it works fine thanks for iterating! Except my question for the sorting of the list of all executables this looks good to me

[dirk-thomas]

The script could print each line as pkg-name executable-basename if no package name was specified?

[mikaelarguedas]

yeah that would be great

[dirk-thomas]

Please try the updated patch: 05b8116

[mikaelarguedas]

New patch works fine. Do you think that the help or the output should add explaination of what the output means ? (I mean indicating that its in the form <PACKAGE_NAME> <EXECUTABLE_NAME>)

[dirk-thomas]

Do you think that the help or the output should add explaination of what the output means ?

I would think it should be clear. Please feel free to propose anything specific.

[mikaelarguedas]

I don't have a great idea hence the question. Feel free to merge it as is and we can iterate on it if we find a clarification is needed and a good way to carry the additional information