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

Enable generate_parameter_module through ament_cmake_python #161

Merged

Conversation

pac48
Copy link
Collaborator

@pac48 pac48 commented Dec 17, 2023

This PR adds the functionality to generate a Python parameter module in a Cmake project. See here for the feature request. I also added another Python example that uses this new feature.

@pac48 pac48 requested a review from ChrisThrasher December 17, 2023 23:02
example_cmake_python/README.md Outdated Show resolved Hide resolved
OUTPUT_VARIABLE PYTHON_VERSION
OUTPUT_STRIP_TRAILING_WHITESPACE)

set(PARAM_HEADER_FILE ${CMAKE_INSTALL_PREFIX}/local/lib/${PYTHON_VERSION}/dist-packages/${PROJECT_NAME}/${LIB_NAME}.py)
Copy link
Contributor

Choose a reason for hiding this comment

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

This CMake function creates files in /usr/local/lib? Why do we want to do that?

I also don't think ${CMAKE_INSTALL_PREFIX}/local/lib is the right path if we do know for a fact that this is what we want to do.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What we want to do is install the Python package to a place on the PYTHONPATH. The way ROS 2 goes about this is to add an entry to the PYTHONPATH for every Python package built from source in your workspace. In this context, CMAKE_INSTALL_PREFIX refers to ros_ws/install/my_python_pkg.

Copy link
Contributor

@Timple Timple Feb 22, 2024

Choose a reason for hiding this comment

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

A bit late perhaps, but I'm running into this.
All other modules of a package like msgs and srvs are without the /local.

@tylerjw
Copy link
Contributor

tylerjw commented Dec 19, 2023

This is failing pre-commit. You might need to rebase it, it seems to be about python quotes.

pac48 and others added 5 commits December 19, 2023 09:17
Signed-off-by: Paul Gesel <paul.gesel@picknik.ai>
Signed-off-by: Paul Gesel <paul.gesel@picknik.ai>
Co-authored-by: Chris Thrasher <chrisjthrasher@gmail.com>
Signed-off-by: Paul Gesel <paulgesel@gmail.com>
@pac48 pac48 force-pushed the pr-add-cmake-generate-parameter-module branch from 64eef19 to 98958c5 Compare December 19, 2023 16:18
@tylerjw tylerjw merged commit 55118ab into PickNikRobotics:main Jan 12, 2024
7 checks passed
@pac48 pac48 deleted the pr-add-cmake-generate-parameter-module branch February 22, 2024 16:04
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.

4 participants