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

Add python message generation #362

Merged
merged 7 commits into from
Jul 17, 2023
Merged

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Jul 14, 2023

🎉 New feature

Summary

Adds python message generation using the old message generation framework. This is an alternative to #355 which uses the new framework.

Test it

Run ctest -R basic_TEST.py.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Addisu Z. Taddese <[email protected]>
@azeey azeey requested a review from caguero as a code owner July 14, 2023 16:39
@github-actions github-actions bot added the 🎵 harmonic Gazebo Harmonic label Jul 14, 2023
@azeey
Copy link
Contributor Author

azeey commented Jul 14, 2023

cc @Voldivh

@codecov
Copy link

codecov bot commented Jul 14, 2023

Codecov Report

Merging #362 (80679bd) into main (4372e74) will not change coverage.
The diff coverage is n/a.

❗ Current head 80679bd differs from pull request most recent head 9b14d17. Consider uploading reports for the commit 9b14d17 to get more accurate results

@@           Coverage Diff           @@
##             main     #362   +/-   ##
=======================================
  Coverage   95.43%   95.43%           
=======================================
  Files          10       10           
  Lines        1030     1030           
=======================================
  Hits          983      983           
  Misses         47       47           

@azeey azeey requested a review from Voldivh July 14, 2023 18:25
Copy link
Contributor

@Voldivh Voldivh left a comment

Choose a reason for hiding this comment

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

LGTM! I tried it out locally and seems to be working.

Copy link
Contributor

@Voldivh Voldivh left a comment

Choose a reason for hiding this comment

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

Taking a closer look, I'm not sure why are we deleting everything related to Ruby and replacing it with Python instead of just adding the Python stuff.

@azeey
Copy link
Contributor Author

azeey commented Jul 15, 2023

@Voldivh I've added options for where the python modules should be installed and added a test. Could you take another look?

Taking a closer look, I'm not sure why are we deleting everything related to Ruby and replacing it with Python instead of just adding the Python stuff.

Support for Ruby was being removed in #339, so I went ahead and removed it here as well.

Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Copy link
Contributor

@Voldivh Voldivh left a comment

Choose a reason for hiding this comment

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

Just a couple of minor comments the rest LGTM!

python/CMakeLists.txt Outdated Show resolved Hide resolved
python/CMakeLists.txt Show resolved Hide resolved
Signed-off-by: Addisu Z. Taddese <[email protected]>
Copy link
Contributor

@Voldivh Voldivh left a comment

Choose a reason for hiding this comment

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

LGTM!

@azeey azeey merged commit 73f4a45 into gazebosim:main Jul 17, 2023
4 checks passed
@azeey azeey deleted the python_gen_old_msgs branch July 17, 2023 23:27
azeey added a commit to gazebo-release/gz-msgs10-release that referenced this pull request Jul 17, 2023
gazebosim/gz-msgs#362 removed ruby file
generation and added python generation. This updates the install rules
to reflect that.

Signed-off-by: Addisu Z. Taddese <[email protected]>
azeey added a commit to gazebo-release/gz-msgs10-release that referenced this pull request Jul 18, 2023
gazebosim/gz-msgs#362 removed ruby file
generation and added python generation. This updates the install rules
to reflect that.

Signed-off-by: Addisu Z. Taddese <[email protected]>
@azeey azeey mentioned this pull request Jul 26, 2023
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎵 harmonic Gazebo Harmonic
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants