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

Python Bindings for Publisher, Subscriber and Service Request features. #411

Merged
merged 39 commits into from
Jul 24, 2023

Conversation

Voldivh
Copy link
Contributor

@Voldivh Voldivh commented Jun 20, 2023

🎉 New feature

Closes #387

Summary

This PR creates the python bindings for the publisher, subscriber and request features from the repository. In order to avoid creating a new dependency to be able to create python bindings from protobuf messages, it was decided to use the methods that publishes, subscribes and make services request with serialized messages, i.e, use the methods PublishRaw, SubscribeRaw and RequestRaw. The bindings for these methods are wrapped by python methods in order to give the user an API similar to the one we have in C++.

We would like to acknowledge and give credit to @srmainwaring for his code which served as a reference and inspiration for this implementation. The main difference with this implementation is that we are using the methods that uses serialized messages instead of creating bindings with the pybind11_protobuf library to the non-serialized methods.

Test it

There are some examples that were created as smoke test in order to test out the functionality. In order to use them you would have to:

  1. Compile from source this branch
  2. Update the PYTHONPATH variable:
export PYTHONPATH=$PYTHONPATH:<path_to_ws>/install/lib/python
  1. Run the python script for the example you would like to try out:
python3 <path_to_ws>/src/gz-transport/python/examples/<example_to_run>

Note: The service requester example will only work if there is a server running that listens to /echo topic and expects a gz.messages.StringMsg as the request. One way to do so is to follow the instructions on the examples folder and run the responser executable.

@Voldivh Voldivh changed the title Python Bindings for Publisher, Subscriber and Requester features. Python Bindings for Publisher, Subscriber and Service Request features. Jun 20, 2023
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Jun 20, 2023
@Voldivh Voldivh changed the base branch from gz-transport12 to main June 20, 2023 22:55
@codecov
Copy link

codecov bot commented Jun 20, 2023

Codecov Report

Merging #411 (2c1f3dd) into main (917bf04) will decrease coverage by 0.33%.
The diff coverage is 74.26%.

❗ Current head 2c1f3dd differs from pull request most recent head 5371b41. Consider uploading reports for the commit 5371b41 to get more accurate results

@@            Coverage Diff             @@
##             main     #411      +/-   ##
==========================================
- Coverage   87.43%   87.11%   -0.33%     
==========================================
  Files          60       61       +1     
  Lines        5291     5462     +171     
==========================================
+ Hits         4626     4758     +132     
- Misses        665      704      +39     
Impacted Files Coverage Δ
python/src/transport/_gz_transport_pybind11.cc 74.26% <74.26%> (ø)

... and 4 files with indirect coverage changes

python/CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
python/CMakeLists.txt Outdated Show resolved Hide resolved
python/examples/publisher.py Show resolved Hide resolved
python/examples/publisher.py Outdated Show resolved Hide resolved
python/src/__init__.py Show resolved Hide resolved
python/src/transport/_gz_transport_pybind11.cc Outdated Show resolved Hide resolved
python/src/transport/_gz_transport_pybind11.cc Outdated Show resolved Hide resolved
python/src/transport/_gz_transport_pybind11.cc Outdated Show resolved Hide resolved
python/test/requester_TEST.py Show resolved Hide resolved
python/examples/subscriber.py Outdated Show resolved Hide resolved
python/src/__init__.py Outdated Show resolved Hide resolved
@Voldivh Voldivh marked this pull request as ready for review June 23, 2023 20:41
@Voldivh Voldivh requested a review from caguero as a code owner June 23, 2023 20:41
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

Looks great overall. Most of the comments should be easy to address. Some
comments might be addressed in a follow-up PR.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
python/CMakeLists.txt Show resolved Hide resolved
python/CMakeLists.txt Outdated Show resolved Hide resolved
python/CMakeLists.txt Outdated Show resolved Hide resolved
python/test/requester_TEST.py Outdated Show resolved Hide resolved
python/test/requester_TEST.py Outdated Show resolved Hide resolved
python/test/pubSub_TEST.py Outdated Show resolved Hide resolved
python/test/requester_TEST.py Outdated Show resolved Hide resolved
python/test/requester_TEST.py Outdated Show resolved Hide resolved
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

I have one test failure. Not sure if it's failing for you. Also, I know I asked
you to rebase this to work with new message generation framework in gz-msgs.
But since that's been reverted, could you also revert the change here? I plan
to open a gz-msgs PR that uses the old message generation framework and adds
python bindings.

python/test/options_TEST.py Outdated Show resolved Hide resolved
python/test/pubSub_TEST.py Show resolved Hide resolved
python/test/pubSub_TEST.py Show resolved Hide resolved
@Voldivh
Copy link
Contributor Author

Voldivh commented Jul 14, 2023

I have one test failure. Not sure if it's failing for you. Also, I know I asked you to rebase this to work with new message generation framework in gz-msgs. But since that's been reverted, could you also revert the change here? I plan to open a gz-msgs PR that uses the old message generation framework and adds python bindings.

No problem, just one question then, should I change this PR to target the main branch as before then?

@azeey
Copy link
Contributor

azeey commented Jul 14, 2023

I have one test failure. Not sure if it's failing for you. Also, I know I asked you to rebase this to work with new message generation framework in gz-msgs. But since that's been reverted, could you also revert the change here? I plan to open a gz-msgs PR that uses the old message generation framework and adds python bindings.

No problem, just one question then, should I change this PR to target the main branch as before then?

Yes, target the main branch.

@Voldivh Voldivh changed the base branch from mjcarroll/rework-message-generation to main July 14, 2023 20:37
@azeey
Copy link
Contributor

azeey commented Jul 18, 2023

@osrf-jenkins run tests please

@azeey
Copy link
Contributor

azeey commented Jul 18, 2023

Needs gazebo-release/gz-msgs10-release#6 for CI.

@azeey azeey force-pushed the voldivh/pybind11_bindings branch from e890c71 to e1bca5e Compare July 19, 2023 21:13
@azeey
Copy link
Contributor

azeey commented Jul 19, 2023

I've built gz-msgs10 debians just for Jammy for testing and added a new dependency in e1bca5e. It looks like tests are passing on Jenkins 🎉

@azeey
Copy link
Contributor

azeey commented Jul 20, 2023

gazebo-release/gz-msgs10-release#6 is merged and I've built the debs for Focal and Jammy, so I'll run tests again.

@osrf-jenkins run tests please

@azeey
Copy link
Contributor

azeey commented Jul 20, 2023

@osrf-jenkins run tests please

@azeey azeey requested a review from ahcorde July 21, 2023 05:37
Copy link
Contributor

@azeey azeey left a comment

Choose a reason for hiding this comment

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

@ahcorde All your feedback has been resolved. Do you want to take another look?

@azeey azeey merged commit da3c55d into gazebosim:main Jul 24, 2023
4 checks passed
@azeey azeey deleted the voldivh/pybind11_bindings branch July 24, 2023 17:32
@azeey azeey mentioned this pull request Aug 29, 2023
@Voldivh Voldivh mentioned this pull request Oct 3, 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.

Add Python bindings
4 participants