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 Wrapper #132

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

progtologist
Copy link

This adds python wrappers to abb_librws using boost.python
We made this compatible for both python2 and python3 (compiles and imports correctly in a catkin workspace regardless of which python version you are using) but please also test it in different distros and setups to verify that it works.
Included there is a ABB robot module and a python test script that you can use to verify the functionality of the wrapper for your setup.

To run the test script in a catkin workspace you can use rosrun (though it is not required)

rosrun --prefix 'python2' abb_librws test_python.py 192.168.0.10
rosrun --prefix 'python3' abb_librws test_python.py 192.168.0.10

To run it without rosrun, you must have sourced the catkin workspace (to extent PYTHONPATH)

python2 src/abb_librws/test/test_python.py 192.168.0.10
python3 src/abb_librws/test/test_python.py 192.168.0.10

@gavanderhoorn
Copy link
Member

Thanks for the PR 👍 this looks really nice, and I've always had a Python binding for abb_librws on my TODO list (which is too long, so I never got to it).

Both @jontje and me are rather busy (ROS-Industrial Conference next week), so it'll take some time for us to review this properly.

Finally: this is going to sound ungrateful, but it would be great if we could avoid adding more Boost dependencies to abb_librws. Modern C++ has many of the things we use here, and even though this isn't a ROS package, we do like the idea of trying to minimise the Boost dependency where possible.

Did you have a particular reason not to use pybind11 for this?

@jontje
Copy link
Contributor

jontje commented Dec 11, 2020

This looks quite promising after a short skim through! 👍

Both @jontje and me are rather busy (ROS-Industrial Conference next week), so it'll take some time for us to review this properly.

I also have another demo to finalize before year-end (in addition to the stuff at the ROS-Industrial Conference 2020).

@progtologist
Copy link
Author

Finally: this is going to sound ungrateful, but it would be great if we could avoid adding more Boost dependencies to abb_librws. Modern C++ has many of the things we use here, and even though this isn't a ROS package, we do like the idea of trying to minimise the Boost dependency where possible.

Did you have a particular reason not to use pybind11 for this?

The main reason we did not use pybind11 is basically xenial support. I have tried to use pybind11 for a different project with g++ 5.5 and C++11 and have always had linking issues in xenial when building for python3.

@gavanderhoorn
Copy link
Member

gavanderhoorn commented Dec 11, 2020

ROS Kinetic will be EOL in about 4 months.

I don't know about @jontje, but I would not necessarily be too distraught if we lose Xenial compatibility.

Users who really want to use that should be able to install an updated compiler.

And we could always disable building the Python-bindings if a compiler which is too old is detected.


Same goes for Python 2 support. Python 2 has been EOL for a long time now.

@gavanderhoorn gavanderhoorn added the enhancement New feature or request label Dec 11, 2020
@progtologist
Copy link
Author

ROS Kinetic will be EOL in about 4 months.

I don't know about @jontje, but I would not necessarily be too distraught if we lose Xenial compatibility.

Users who really want to use that should be able to install an updated compiler.

These are all fair, but for our purposes (at Unibap), we still support Xenial and given that python2 is the default python for that version, we support that as well, and that drove our implementation towards Boost.Python.

And we could always disable building the Python-bindings if a compiler which is too old is detected.

Same goes for Python 2 support. Python 2 has been EOL for a long time now.

The CMakeLists.txt provided in the PR silently disables python bindings for each python version that Boost.Python is not found.

@mjd3 mjd3 mentioned this pull request Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

4 participants