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

Support ament_cmake_python for pkg create #904

Closed
wants to merge 3 commits into from
Closed

Support ament_cmake_python for pkg create #904

wants to merge 3 commits into from

Conversation

m12watanabe1a
Copy link

@m12watanabe1a m12watanabe1a commented Apr 29, 2024

About this PR

  • Implement new argument --template-name for selecting different template
  • Giving '?' shows the list of the available template
  • Support new template ament_cmake_python

Related Issue

#739

How to use

ros2 pkg create --build-type ament_cmake --template-name ament_cmake_python my_project

This command will genrate,

.
└── my_project
    ├── CMakeLists.txt
    ├── package.xml
    └── my_project
        └── __init__.py

Both <node_name> or <library_name> given in the cli will generate node or library in cpp.

Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I can see, there is no package type called ament_cmake_python. For all of our python packages that are built using CMake, the package.xml type is just ament_cmake (one example)

@m12watanabe1a Can you explain more about what you are trying to accomplish here?

@m12watanabe1a
Copy link
Author

m12watanabe1a commented Apr 29, 2024

@clalancette Thank you for reviewing.

I tired to newly support for ament_cmake_python in ros2 pkg create command since there is a demand in the linked issue.

#739

Feature description
ROS2 CLI allows creating packages for cmake, ament_cmake, ament_python. Considering there isn't a complete example in the documentation for ament_cmake_python , it would be helpful to support generating one for users.

Signed-off-by: m12watanabe1a <[email protected]>
@@ -112,6 +112,10 @@ def create_package_environment(package, destination_directory):
if package.get_build_type() == 'ament_python':
print('creating source folder')
source_directory = _create_folder(package.name, package_directory)
if package.get_build_type() == 'ament_cmake_python':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are currently no instances of ROS 2 packages where this would be true. That is, package.get_build_type() will never return ament_cmake_python.

That said, it would still be worthwhile to have some way to create a package template for a Python package using CMake. However, it will have to be done a different way because it will use a build_type of ament_cmake. Maybe we can add in a new command-line argument that tells it to generate Python installation stuff in the CMakeLists.txt?

Copy link
Author

@m12watanabe1a m12watanabe1a Apr 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can add in a new command-line argument that tells it to generate Python installation stuff in the CMakeLists.txt?

I see. I agree with that build_type is not appropriate field for generating templates.

So maybe something like like,

ros2 pkg create --build-type ament_cmake --template-name ament_cmake_python <package_name>

@m12watanabe1a m12watanabe1a requested a review from clalancette May 1, 2024 02:47
@sloretz
Copy link
Contributor

sloretz commented May 9, 2024

I added an agenda item to the next ROS 2 meeting to get feedback on the naming of --template-name , afterwards the ros2cli maintainers will be assigned to review. 🧇

Thank you for your patience!

@m12watanabe1a
Copy link
Author

Thank you so much 🙏

@m12watanabe1a m12watanabe1a closed this by deleting the head repository Sep 30, 2024
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.

3 participants