[WIP] Bebop/RPi Toolchain Update v2 #7248

nicolaerosia · 2017-05-15T20:38:08Z

This is a continuation of #6528

You will need to set PATH to a include the path to a arm-linux-gnueabihf-g++ compiler

GCC Testing:
a. get rpi-tools, git clone https://github.com/raspberrypi/tools

export PATH=<path>/tools/arm-bcm2708/gcc-linaro-arm-linux-gnueabihf-raspbian-x64/bin/:$PATH
export PX4_SOURCE=<px4-source>

b. get a different compiler, like 5.3.1 from Linaro: https://releases.linaro.org/components/toolchain/binaries/5.3-2016.05/arm-linux-gnueabihf/gcc-linaro-5.3.1-2016.05-x86_64_arm-linux-gnueabihf.tar.xz

export PATH=gcc-path>/bin:$PATH
export PX4_SOURCE=<px4-source>

cmake \
-G"Unix Makefiles" \
-DCONFIG=posix_rpi_cross \
${PX4_SOURCE}

make

Clang Testing:
You will need a GCC cross-compiler!
a. get prebuild clang, http://releases.llvm.org/download.html

wget http://releases.llvm.org/3.9.0/clang+llvm-3.9.0-x86_64-fedora23.tar.xz

unpack

Create symlinks for clang and clang++ in gcc cross-compiler dir:

ln -s <clang-path>/bin/clang <gcc-path>/bin/clang
ln -s <clang-path>/bin/clang++ <gcc-path>/bin/clang++
export PATH=<gcc-path>/bin:$PATH

export PX4_SOURCE=<px4-source>

cmake \
-G"Unix Makefiles" \
-DCONFIG=posix_rpi_cross \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
${PX4_SOURCE}

make

@mhkabir @dagar could you please take a look?

mhkabir · 2017-05-15T20:39:25Z

Nice! Thanks for taking this up!

nicolaerosia · 2017-05-15T20:43:58Z

Let me know if you are OK with the approach, I tried to simplify it and it make it easy to change compilers by not hardcoding much.

dagar · 2017-05-15T21:10:32Z

Do we need the environment variable PX4_CXX_COMPILER at all? It's generally not a good fit for cmake because various things can cause it to rerun configure right before an incremental build when that environment variable may no longer be set.

dagar · 2017-05-15T21:13:37Z

Tools/adb_upload_to_bebop.sh

@@ -34,7 +34,7 @@ adb shell mount -o remount,rw /
 adb shell touch /home/root/parameters
 adb shell mkdir -p /data/ftp/internal_000/fs/microsd

-${RPI_TOOLCHAIN_DIR}/gcc-linaro-arm-linux-gnueabihf/bin/arm-linux-gnueabihf-strip \
+${PX4_TOOLCHAIN_DIR}/gcc-linaro-arm-linux-gnueabihf/bin/arm-linux-gnueabihf-strip \


Why bother stripping before upload? Either let cmake do this (it can find arm-linux-gnueabihf-strip) or consider not bothering.

Bebop needed the strip for the executable to live on the system partition. Was too big otherwise.

nicolaerosia · 2017-05-15T21:14:48Z

@dagar it was PX4_TOOLCHAIN_DIR before. I would rather get rid of env variables, I've tried using -DCMAKE_CXX_COMPILER without success, for some reason the Toolchain-arm-linux-gnueabihf.cmake file is being parsed twice. Any ideas?

nicolaerosia · 2017-05-15T21:24:12Z

@dagar , I've changed ii, but since everyone uses "make" in PX4_SOURCE_DIR, we will have to patch the Makefile which invokes CMake to pass the path to the compiler...

new usage:

cmake \
-DCMAKE_INSTALL_PREFIX=${PX4_INSTALL} \
-DCMAKE_CXX_COMPILER=$HOME/aero-workspace/gcc-linaro-5.3.1-2016.05-x86_64_arm-linux-gnueabihf/bin/clang++ \
-G"Unix Makefiles" \
-DCONFIG=posix_rpi_cross \
${PX4_SOURCE}

dagar · 2017-05-15T21:32:39Z

Maybe I'm missing something, but I don't see why setting up the environment and building PX4 aren't completely separate things.

"install" (extract) your desired toolchain and add it to your PATH
let PX4 cmake run normally and it will find arm-linux-gnueabihf-gcc, etc

dagar · 2017-05-15T21:35:43Z

Tools/adb_upload_to_bebop.sh

+    restart_px4=true
+fi
+
+`dirname ${PX4_CXX_COMPILER}`/arm-linux-gnueabihf-strip \
  -R .comment -R .gnu.version \
  ../build_posix_bebop_default/src/firmware/posix/px4


Do this as a cmake custom command as it will already have the binary path and arm-linux-gnueabihf-strip.

custom target like 'make strip' ?

Kind of like this (https://github.com/PX4/Firmware/blob/master/src/firmware/posix/sitl_tests.cmake#L59), but with add_custom_command (https://cmake.org/cmake/help/v3.0/command/add_custom_command.html).

dagar · 2017-05-15T21:37:52Z

cmake/configs/posix_rpi_common.cmake

@@ -8,6 +8,7 @@ include(posix/px4_impl_posix)
 add_definitions(
 	-D__PX4_POSIX_RPI
 	-D__DF_LINUX # For DriverFramework
+	-D__DF_RPI # For DriverFramework


These should be pushed into https://github.com/PX4/DriverFramework

Where exactly? I see some files in https://github.com/PX4/DriverFramework/tree/master/cmake/toolchains which are not used anywhere in the project.

Perhaps we should make a note of it in that repo and come back to it. It seems to me that the DriverFramework cmake should be capable of determining this independently. Especially if there's any value in DF outside of PX4.

dagar · 2017-05-15T21:39:53Z

cmake/posix/px4_impl_posix.cmake

@@ -267,22 +267,32 @@ elseif ("${BOARD}" STREQUAL "excelsior")
 		--sysroot=${HEXAGON_ARM_SYSROOT}/lib32-apq8096  -mfloat-abi=softfp -mfpu=neon -mthumb-interwork

 		)
-elseif ("${BOARD}" STREQUAL "rpi" AND "$ENV{RPI_USE_CLANG}" STREQUAL "1")
+elseif ("${BOARD}" STREQUAL "rpi")
+	SET(RPI_COMPILE_FLAGS


Shouldn't this live in the toolchain file or does the existing structure force this?

The compiler is not only for RPI, it can be for any ARM CPU, here we are only handling RPI (RPI v3 actually)

nicolaerosia · 2017-05-15T21:59:21Z

@dagar ok, that works. Any idea on how to handle Clang ?

dagar · 2017-05-15T22:32:06Z

Doesn't that make sense as a separate toolchain file for clang in this situation? Then use find_program to get the arm-linux-gnueabihf path.

nicolaerosia · 2017-05-16T05:52:00Z

Alright, but how do you tell the build system to pick Clang? We can either add a new target (not desirable), use an environmental variable (again, not desirable), or we could just let the user specify CMAKE_TOOLCHAIN_FILE, and consider this as an advanced configuration.

dagar · 2017-05-16T16:36:29Z

Similar issue here - #6892

Some possible solutions

Add another cmake config *_clang.cmake for each target where you might want to use clang
Single toolchain with a cmake option to enable clang (off by default)
Single toolchain and try to be clever respecting if the user set CMAKE_CXX_COMPILER to clang

nicolaerosia · 2017-05-16T16:47:52Z

@dagar I went with option 3, to fix the CI we need to add the compiler path to PATH, could you help with that please?
If you are ok with the approach, I will squash the patches.

dagar · 2017-05-16T16:50:13Z

Yes that's fine from my perspective.
Are you familiar with the PX4 docker images? Here's the one used to build raspi and bebop -
https://github.com/PX4/containers/blob/master/docker/px4-dev/Dockerfile_raspi

nicolaerosia · 2017-05-16T16:51:46Z

I'm not, should I create a PR there?

nicolaerosia · 2017-05-16T17:01:01Z

I created a target for @mhkabir strip use case. @dagar you said you want a custom_command, not custom_target, could you provide more details?

dagar · 2017-05-16T17:08:09Z

Yes please update the docker file, or I could do it later if you aren't familiar with docker.

The idea with add_custom_command is you could let cmake handle the dependencies, find the required tools, and use the path to the binary it already knows. CMake custom targets are always considered out of date and therefore run everytime called.

Something like this

add_custom_command(OUTPUT px4.stripped
                   COMMAND arm-linux-gnueabihf-strip -R .comment -R .gnu.version $<TARGET_FILE:px4>
                   DEPENDS px4)

Then the upload could be another dependent custom target that depends on px4.stripped.

nicolaerosia · 2017-05-16T17:22:42Z

@dagar, hopefully I got it right: PX4/PX4-containers#64
I've added the custom_command and custom_target

dagar · 2017-05-16T19:46:31Z

I'll give this a try within the updated docker image. Updating semaphore is awkward because the configuration isn't stored within the repository.

Thanks for making all these little changes. Getting the structure as simple as possible should help with future platforms and maintenance.

nicolaerosia · 2017-05-18T15:10:34Z

gentle ping

dagar · 2017-05-18T15:19:50Z

We're trying to get a v1.6.0 stable release out, but this can go in right after. I need to find a solution for removing the specific docker container from the non-versioned semaphore config.

I was thinking of updating the docker_run.sh script the CI system uses (https://github.com/PX4/Firmware/blob/master/Tools/docker_run.sh) to pick the correct docker image based on input.

nicolaerosia · 2017-05-18T17:45:25Z

no worries. Should I squash my commits ?

dagar · 2017-05-31T18:52:20Z

src/firmware/posix/CMakeLists.txt

@@ -138,4 +138,14 @@ install(TARGETS px4 DESTINATION ${CMAKE_INSTALL_BINDIR})
 install(DIRECTORY ${PROJECT_SOURCE_DIR}/ROMFS DESTINATION ${CMAKE_INSTALL_DATADIR}/${PROJECT_NAME})
 install(DIRECTORY ${PROJECT_SOURCE_DIR}/posix-configs DESTINATION ${CMAKE_INSTALL_DATADIR}/${PROJECT_NAME})

+FIND_PROGRAM(STRIP_TOOL "arm-linux-gnueabihf-strip")


Shouldn't this be in the Toolchain?

dagar · 2017-05-31T18:52:42Z

src/firmware/posix/CMakeLists.txt

+)
+
+add_custom_command(OUTPUT px4.stripped
+			COMMAND arm-linux-gnueabihf-strip -R .comment -R .gnu.version $<TARGET_FILE:px4>


then use ${STRIP_TOOL} instead of arm-linux-gnueabihf-strip

dagar · 2017-05-31T20:45:29Z

Oh, we need to tag your docker changes.

dagar · 2017-05-31T20:48:01Z

It'll probably take the better part of an hour for docker hub to build, but once it does can you push a commit to your branch that changes the tag here? https://github.com/PX4/Firmware/blob/master/Tools/docker_run.sh#L8

https://hub.docker.com/r/px4io/px4-dev-raspi/builds/

but missed to move it from *-clang.cmake Toolchain Signed-off-by: Nicolae Rosia <nicolae.rosia@gmail.com>

Add CMake target for strip since these changes break adb_upload_to_bebop. GCC users should add the cross compiler bin path to PATH (location of arm-linux-gnueabihf-g++). Clang user should do the following: * set CMAKE_CXX_COMPILER to clang++ by providing -DCMAKE_CXX_COMPILER=clang++ to cmake * get GCC cross compiler - needed because Clang does not ship a CRT * create a symlink for clang and clang++ in GCC cross compiler bin dir. * add GCC bin dir to PATH Signed-off-by: Nicolae Rosia <nicolae.rosia@gmail.com>

Signed-off-by: Nicolae Rosia <nicolae.rosia@gmail.com>

nicolaerosia · 2017-06-05T09:22:06Z

@dagar , I have sent a PR for Devguide update, PX4/PX4-Devguide#175

dagar · 2017-06-06T20:46:54Z

src/firmware/posix/CMakeLists.txt

+)
+
+add_custom_command(OUTPUT px4.stripped
+			COMMAND ${STRIP_TOOL} -R .comment -R .gnu.version $<TARGET_FILE:px4>


Do we need the upload custom command to depend on the stripped output instead? https://github.com/nicolaerosia/Firmware/blob/f4ba9c5acb3a0efec823e398e76bbbf976013372/src/firmware/posix/CMakeLists.txt#L103

dagar · 2017-06-06T20:48:52Z

Check the bebop upload dependency, but this looks good to me.
In general I'd like a better idea of how we should structure cmake properly for all these different boards and platforms, but I'll save that for another day.
Clang for NuttX builds was handled a bit differently here. #6892

@bkueng can you review?

bkueng

Looks good to me. Though I have not tested Bebop, only RPi.

Signed-off-by: Nicolae Rosia <nicolae.rosia@gmail.com>

nicolaerosia · 2017-06-07T20:04:26Z

@dagar can we merge this?

dagar · 2017-06-07T20:08:36Z

Is anyone able to test bebop?

bkueng · 2017-06-08T06:03:20Z

Is anyone able to test bebop?

@ChristophTobler or @eyeam3 maybe?

dagar · 2017-06-12T23:48:37Z

According to @eyeam3 in the v1.6 release sign Bebop wasn't functioning. #7258 (comment)

Let's get this in now and revisit once someone is interested in bringing Bebop back to life.

nicolaerosia changed the title ~~Toolchain update~~ [WIP] Bebop/RPi Toolchain Update v2 May 15, 2017

dagar reviewed May 15, 2017

View reviewed changes

mhkabir self-requested a review May 16, 2017 07:14

nicolaerosia mentioned this pull request May 16, 2017

Add toolchain path to PATH PX4/PX4-containers#64

Merged

dagar added this to the Release v1.7.0 milestone May 18, 2017

dagar added Admin: Enhancement (improvement) 💡 Board: raspberry pi 🍓 labels May 18, 2017

nicolaerosia mentioned this pull request May 30, 2017

[WIP] Bebop/RPi Toolchain Update #6528

Closed

dagar reviewed May 31, 2017

View reviewed changes

nicolaerosia mentioned this pull request Jun 5, 2017

Update Raspberry Pi dev env setup to be in sync with toolchain updates PX4/PX4-Devguide#175

Merged

mhkabir and others added 9 commits June 5, 2017 10:30

Rename to generic arm toolchain

e229153

Rename RPi/Bebop toolchain locations to PX4_TOOLCHAIN_DIR

4acc5d0

Relocate DF_RPI define

524e7c2

bebop uploader : kill and restart autostarted PX4 daemon

e24329b

Commit 8797a090dc06cd0c9f4cf132b67e4872af1ff8f1 relocated __DF_RPI

7456565

but missed to move it from *-clang.cmake Toolchain Signed-off-by: Nicolae Rosia <nicolae.rosia@gmail.com>

navio_adc: add override flag to init

0861e72

Signed-off-by: Nicolae Rosia <nicolae.rosia@gmail.com>

Move find of STRIP_TOOL to toolchain

13f832b

Signed-off-by: Nicolae Rosia <nicolae.rosia@gmail.com>

update docker_run.sh

f4ba9c5

Signed-off-by: Nicolae Rosia <nicolae.rosia@gmail.com>

dagar reviewed Jun 6, 2017

View reviewed changes

dagar requested a review from bkueng June 6, 2017 20:49

dagar previously approved these changes Jun 6, 2017

View reviewed changes

bkueng previously approved these changes Jun 7, 2017

View reviewed changes

Fix strip target and bebop upload

475ae4d

Signed-off-by: Nicolae Rosia <nicolae.rosia@gmail.com>

nicolaerosia dismissed stale reviews from bkueng and dagar via 475ae4d June 7, 2017 18:27

dagar approved these changes Jun 12, 2017

View reviewed changes

LorenzMeier merged commit 0dc4f1f into PX4:master Jun 13, 2017

[WIP] Bebop/RPi Toolchain Update v2 #7248

[WIP] Bebop/RPi Toolchain Update v2 #7248

Conversation

nicolaerosia commented May 15, 2017 • edited Loading

mhkabir commented May 15, 2017

nicolaerosia commented May 15, 2017

dagar commented May 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolaerosia commented May 15, 2017

nicolaerosia commented May 15, 2017

dagar commented May 15, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolaerosia May 15, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolaerosia commented May 15, 2017

dagar commented May 15, 2017

nicolaerosia commented May 16, 2017

dagar commented May 16, 2017

nicolaerosia commented May 16, 2017 • edited Loading

dagar commented May 16, 2017

nicolaerosia commented May 16, 2017

nicolaerosia commented May 16, 2017

dagar commented May 16, 2017

nicolaerosia commented May 16, 2017

dagar commented May 16, 2017

nicolaerosia commented May 18, 2017

dagar commented May 18, 2017

nicolaerosia commented May 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dagar commented May 31, 2017

dagar commented May 31, 2017 • edited Loading

nicolaerosia commented Jun 5, 2017

Choose a reason for hiding this comment

dagar commented Jun 6, 2017

bkueng left a comment

Choose a reason for hiding this comment

nicolaerosia commented Jun 7, 2017

dagar commented Jun 7, 2017

bkueng commented Jun 8, 2017

dagar commented Jun 12, 2017

nicolaerosia commented May 15, 2017 •

edited

Loading

nicolaerosia May 15, 2017 •

edited

Loading

nicolaerosia commented May 16, 2017 •

edited

Loading

dagar commented May 31, 2017 •

edited

Loading