bitcraze / crazyflie-firmware

The main firmware for the Crazyflie Nano Quadcopter, Crazyflie Bolt Quadcopter and Roadrunner Positioning Tag.
GNU General Public License v3.0
1.21k stars 1.06k forks source link

release the python bindings as pip package #1067

Open knmcguire opened 2 years ago

knmcguire commented 2 years ago

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 commented 2 years ago

This is fairly easy to do with github actions. See https://github.com/bitcraze/crazyflie-link-cpp/blob/master/.github/workflows/publish.yaml

knmcguire commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

knmcguire commented 1 year ago

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_dirsthat we give to the Extension object in setup.py: https://github.com/bitcraze/crazyflie-firmware/blob/af96c070d6bc32cf73eeb89980a970d7e01f7f8c/bindings/setup.py#L35-L45

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 commented 1 year ago

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 https://github.com/bitcraze/crazyflie-firmware/blob/af96c070d6bc32cf73eeb89980a970d7e01f7f8c/src/utils/interface/lighthouse/lighthouse_types.h#L12

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? https://github.com/bitcraze/crazyflie-firmware/blob/af96c070d6bc32cf73eeb89980a970d7e01f7f8c/bindings/cffirmware.i#L5

whoenig commented 1 year ago

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

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

knmcguire commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

We will have a meeting about these python bindings soon

kam56 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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.