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

release the python bindings as pip package #1067

Open
knmcguire opened this issue Jun 22, 2022 · 16 comments
Open

release the python bindings as pip package #1067

knmcguire opened this issue Jun 22, 2022 · 16 comments

Comments

@knmcguire
Copy link
Member

If we release the python bindings of the firmware for every firmware release, we could use the functions in external projects more easily, like in the controller of Webots.

@whoenig
Copy link
Contributor

whoenig commented Jul 5, 2022

@knmcguire
Copy link
Member Author

ah good! Thanks!

you don't think there will be any OS problems? That if the bindings is build for Ubuntu that they won't work for window for instance?

@whoenig
Copy link
Contributor

whoenig commented Jul 5, 2022

You have to build for each platform you want to support a separate wheel, which is uploaded to pip. The CI does take care of that, though (in the build part of it: https://github.com/bitcraze/crazyflie-link-cpp/blob/master/.github/workflows/CI.yml).

@knmcguire
Copy link
Member Author

ah great. I'll have a look at the actions for that. And keep it in mind for the next release, which will probably be in September.

@knmcguire
Copy link
Member Author

We will not do this for release 2022.09 due to lack of time, but a potential candidate for a release after that

@knmcguire
Copy link
Member Author

So I've looked at this more closely and with some trouble I need to conclude unfortunately it seems to be not as simple as copying the same structure as https://github.com/bitcraze/crazyflie-link-cpp. For Linux, and probably mac as well, it should be relatively simple with the following steps:

make cf2_defconfig && make
swig -python -Isrc/modules/interface -Isrc/hal/interface -Isrc/utils/interface -o build/cffirmware_wrap.c bindings/cffirmware.i
python3 bindings/setup.py bdist_wheel

But windows of course, this makes it more difficult. Building the firmware natively on windows is a pain so I'm using the bitcraze builder docker for that:

docker run -it -v %CD%:/module bitcraze/builder make cf2_defconfig
docker run -it -v %CD%:/module bitcraze/builder make
swig -python -Isrc/modules/interface -Isrc/hal/interface -Isrc/utils/interface -o build/cffirmware_wrap.c bindings/cffirmware.i
python bindings/setup.py bdist_wheel

First of all, the "-Wno-address-of-packed-member" can not be used but that is okay to remove as that only creates warnings if I'm correct. So after disabling that I get the following types of errors, which does not happen in Ubuntu.

build/cffirmware_wrap.c(18254): error C2059: syntax error: ')'
build/cffirmware_wrap.c(18267): error C2065: 'arg1': undeclared identifier
build/cffirmware_wrap.c(18267): error C2065: 'sweepAngleMeasurement_t': undeclared identifier

But... these all seem to be part of the include_dirs that we give to the Extension object in setup.py:

cffirmware = Extension(
"_cffirmware",
include_dirs=include,
sources=fw_sources + ["build/cffirmware_wrap.c"],
extra_compile_args=[
"-O3",
# The following flags are also used for compiling the actual firmware
"-fno-strict-aliasing",
"-Wno-address-of-packed-member",
],
)

Btw, python3 bindings/setup.py sdist has no problem running but that tar.gz file does not include the header files either.

There is an handy stackoverflow comment here: https://stackoverflow.com/questions/71183800/how-to-include-header-file-in-source-distribution. Let's see if that gets me closer.

@knmcguire
Copy link
Member Author

Another input. looking through all the errors, I see that earlier it does read out the header files:

src/utils/interface/lighthouse\lighthouse_types.h(12): error C2143: syntax error: missing ')' before '('src/utils/interface/lighthouse\lighthouse_types.h(12): error C2059: syntax error: ')' src/utils/interface/lighthouse\lighthouse_types.h(12): error C2146: syntax error: missing ')' before identifier 'lighthouseCalibrationSweep_t'

This location corresponds with

} __attribute__((packed)) lighthouseCalibrationSweep_t;

It seems that windows compile c code with visual c++ and that doesn't use __attribute__(()), so that has to be replaced with something else, namely __declspec(). I found this stack overflow that explains that: https://stackoverflow.com/questions/28411283/dealing-with-attribute-in-msvc

But... wasn't this line supposed to take care of that?

#define __attribute__(x)

@whoenig
Copy link
Contributor

whoenig commented Jan 6, 2023

Would it be possible to use gcc/clang on Windows?

That line probably has issues with the multiple brackets __attribute__((packed)) -> __attribute__(packed)?

@knmcguire
Copy link
Member Author

Ah no, the brackets are not the issue I'm afraid. I haven't tried another compiler yet so I'll try that now.

@knmcguire
Copy link
Member Author

So I couldn't figure out how to force bdist_wheel to use clang for python bindings\setup.py bdist_wheel

But this compiles for the .so file.
python bindings/setup.py build_ext --inplace --compiler=mingw32

but then I get a linker problem

build\temp.win-amd64-cpython-310\Release\build\_cffirmware.cp310-win_amd64.def : fatal error LNK1107: invalid or corrupt file: cannot read at 0x46
clang: error: linker command failed with exit code 1107 (use -v to see invocation)
error: command 'C:\\Program Files\\LLVM\\bin\\clang.exe' failed with exit code 110

@ataffanel
Copy link
Member

It should be possible to use GCC (ie. mingw) on Windows. I did a quick test in a VM to compile a 'hello world' python C extension and it worked, I could build a wheel. I followed instructions found on stack overflow and using w64devkit as mingw distribution.

I do not have time to go further today but it does looks good mostly because w64devkit comes with a shell (busybox) and GNU Make, so it should be a good environment to compile the Crazyflie firmware.

@ataffanel
Copy link
Member

ataffanel commented Jan 12, 2023

I think I have a plan to go forward with this: Building a source distribution (ie. the result of python setup.py sdist) is working. A source distribution is also not such a bad idea since it will allow platform we do not compile for to have a chance to use the binding (I am thinking about arm platform like M1 mac and raspi...). Pip installing a source package works as long as gcc and the dev files for python are installed.

So, since the main issue on Windows is that we cannot run the Crazyflie build system, my plan is for the CI to generate and publish a proper source package and then build windows/mac/Linux wheels out of this package. It does feel cleaner (everything is built from the same source) as well as allowing build on Windows.

The current problem is to persuade python/distutils to include all the necessary files in the source distribution. Currently sdist works, but do not include all that is required for the build.

@knmcguire
Copy link
Member Author

We will have a meeting about these python bindings soon

@kam56
Copy link

kam56 commented Mar 24, 2023

I get the following error on mac while trying to generate python bindings. Any suggestions on how to solve it?

clang: error: no such file or directory: 'vendor/CMSIS/CMSIS/DSP/Source/BasicMathFunctions/arm_add_f32.c'
clang: error: no input files
error: command '/usr/bin/clang' failed with exit code 1

@krichardsson
Copy link
Contributor

@kam56 This sounds more like a build problem and not really related to this issue. Please start a new thread in our discussions instead

@kam56
Copy link

kam56 commented Mar 27, 2023

@krichardsson Thank you for the reply. I have created a new thread .
Please have a look. I am using the Crazyflie simulator for my thesis research. And this error has stopped me from proceeding. I would be grateful for any suggestions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants