search

raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.59k stars 891 forks source link

How to use CMake functionality with other libraries that depend on SDK #1234

Closed cinderblock closed 1 year ago

cinderblock commented 1 year ago

I've got a basic simple program working with the SDK. I'm trying to add FastLED to control a string of pixels. I'm a little new to CMake so maybe I'm doing something silly, but I'm pretty familiar with other build chains so I know what needs to happen high level.

My CMakeLists.txt:

cmake_minimum_required(VERSION 3.13)

include(lib/SDK/pico_sdk_init.cmake)

project(my_project)

# initialize the Raspberry Pi Pico SDK
pico_sdk_init()

add_executable(my_executable
    src/main.cpp
)

# Add pico_stdlib library which aggregates commonly used features
target_link_libraries(my_executable PUBLIC
    pico_stdlib
    hardware_spi
    hardware_i2c

    # hardware_sync
    # hardware_gpio
    # hardware_clocks
    # hardware_dma
    # hardware_pio
    # cmsis_core
    # pico_malloc
    # pico_time

    FastLED
)

add_subdirectory(lib/FastLED)

# create map/bin/hex/uf2 file in addition to ELF.
pico_add_extra_outputs(my_executable)

Unfortunately, since FastLED doesn't directly support CMake, I need to make my own lib/FastLED/CMakeLists.txt for it:

add_library(FastLED STATIC
    FastLED/src/colorpalettes.cpp
    FastLED/src/colorutils.cpp
    FastLED/src/FastLED.cpp
    FastLED/src/hsv2rgb.cpp
    FastLED/src/platforms.cpp
)
target_link_libraries(FastLED PRIVATE
    hardware_sync
    hardware_gpio
    hardware_clocks
    hardware_dma
    hardware_pio
    cmsis_core
    pico_malloc
    pico_time
)
# include `compat.hpp` when building all FastLED sources (replaces `Arduino.h`)
target_compile_options(FastLED PRIVATE -include ${CMAKE_CURRENT_SOURCE_DIR}/compat.hpp)

I can select the sources I want in add_library. However they don't compile without also adding the shown target_link_libraries. This makes sense to me - CMake is using this information to tell the compiler where pico-sdk's include directories are and we can build from there.

The problem that comes up is that these dependencies are then built with the extra target_compile_options which breaks the compilation, notably of pico-sdk's Assembly sources:

[build] FAILED: lib/FastLED/CMakeFiles/FastLED.dir/__/SDK/src/rp2_common/hardware_irq/irq_handler_chain.S.obj 
[build] /usr/bin/arm-none-eabi-gcc -DLIB_CMSIS_CORE=1 -DLIB_PICO_MALLOC=1 -DLIB_PICO_PLATFORM=1 -DLIB_PICO_SYNC=1 -DLIB_PICO_SYNC_CORE=1 -DLIB_PICO_SYNC_CRITICAL_SECTION=1 -DLIB_PICO_SYNC_MUTEX=1 -DLIB_PICO_SYNC_SEM=1 -DLIB_PICO_TIME=1 -DLIB_PICO_UTIL=1 -DPICO_BOARD=\"pico\" -DPICO_BUILD=1 -DPICO_NO_HARDWARE=0 -DPICO_ON_DEVICE=1 -I../lib/SDK/src/rp2_common/hardware_sync/include -I../lib/SDK/src/common/pico_base/include -Igenerated/pico_base -I../lib/SDK/src/boards/include -I../lib/SDK/src/rp2_common/pico_platform/include -I../lib/SDK/src/rp2040/hardware_regs/include -I../lib/SDK/src/rp2_common/hardware_base/include -I../lib/SDK/src/rp2040/hardware_structs/include -I../lib/SDK/src/rp2_common/hardware_claim/include -I../lib/SDK/src/rp2_common/hardware_gpio/include -I../lib/SDK/src/rp2_common/hardware_irq/include -I../lib/SDK/src/common/pico_sync/include -I../lib/SDK/src/common/pico_time/include -I../lib/SDK/src/rp2_common/hardware_timer/include -I../lib/SDK/src/common/pico_util/include -I../lib/SDK/src/rp2_common/hardware_clocks/include -I../lib/SDK/src/rp2_common/hardware_resets/include -I../lib/SDK/src/rp2_common/hardware_pll/include -I../lib/SDK/src/rp2_common/hardware_vreg/include -I../lib/SDK/src/rp2_common/hardware_watchdog/include -I../lib/SDK/src/rp2_common/hardware_xosc/include -I../lib/SDK/src/rp2_common/hardware_dma/include -I../lib/SDK/src/rp2_common/hardware_pio/include -I../lib/SDK/src/rp2_common/cmsis/stub/CMSIS/Core/Include -I../lib/SDK/src/rp2_common/cmsis/stub/CMSIS/Device/RaspberryPi/RP2040/Include -I../lib/SDK/src/rp2_common/pico_malloc/include -mcpu=cortex-m0plus -mthumb -Og -g -include /home/pi/pico/lib/FastLED/compat.hpp -o lib/FastLED/CMakeFiles/FastLED.dir/__/SDK/src/rp2_common/hardware_irq/irq_handler_chain.S.obj   -c ../lib/SDK/src/rp2_common/hardware_irq/irq_handler_chain.S
[build] /usr/lib/gcc/arm-none-eabi/8.3.1/include/stddef.h: Assembler messages:
[build] /usr/lib/gcc/arm-none-eabi/8.3.1/include/stddef.h:216: Error: bad instruction `typedef unsigned int size_t'
[build] /usr/lib/gcc/arm-none-eabi/8.3.1/include/stddef.h:328: Error: bad instruction `typedef unsigned int wchar_t'
[build] /usr/lib/gcc/arm-none-eabi/8.3.1/include/stddef.h:149: Error: bad instruction `typedef int ptrdiff_t'
[build] /usr/lib/gcc/arm-none-eabi/8.3.1/include/stddef.h:357: Error: bad instruction `typedef unsigned int wint_t'
...

I thought by specifying the libraries at the top level it would force them to be compiled there with those options. It seems they do, but don't replace the similar compilation in the dependency.

I suspect I'm missing some simple option that means just "get the include directories of these dependencies". But I've tried many permutations of target_link_libraries's (and other functions') options to no avail.

I've even tried manually specifying the include directories that are missing with something like:

target_include_directories(FastLED PRIVATE
    ../SDK/src/rp2_common/hardware_sync/include
    ../SDK/src/rp2_common/hardware_gpio/include
    ../SDK/src/rp2_common/hardware_clocks/include
    ../SDK/src/rp2_common/hardware_dma/include
    ../SDK/src/rp2_common/hardware_pio/include
    ../SDK/src/rp2_common/cmsis_core/include
    ../SDK/src/rp2_common/pico_malloc/include
    ../SDK/src/common/pico_time/include
    ../SDK/src/common/pico_base/include
)

That gets pretty far but breaks down at finding version.h, generated from version.h.in.

I've looked through the SDK documentation and examples but rarely found more detail on the pico sdk/cmake expects me to tell it what it needs to know. Any thoughts on moving forward here would be greatly appreciated.

For completeness compat.hpp:

#include <stdlib.h>
#include "pico/time.h"
#include "RP2040.h"

// FastLED tests for this to enable RP2040 code
#define ARDUINO_ARCH_RP2040

static inline uint32_t micros(void) { return to_us_since_boot(get_absolute_time()); }
static inline uint32_t millis(void) { return to_ms_since_boot(get_absolute_time()); }
static inline void yield(void) {};
lurch commented 1 year ago

I'm sure Graham will be along with the "proper" answer later, but in the meantime I wonder if any if the comments in #924 help? :man_shrugging:

peterharperuk commented 1 year ago

So this compat.hpp is just needed for the fastled component? Most sdk libraries have a "_headers" version which should avoid your static lib compiling sdk source e.g. hardware_sync has a hardware_sync_headers library that adds the include paths (not that the develop branch has some improvements in this area) . As you've used PRIVATE I wouldn't expect that include compile option to be applied outside the static library. Is that not the case?

kilograham commented 1 year ago

Unless you deliberately wanted to use a static lib (which from your description, you don't), you should use 

add_library(FastLED INTERFACE

and

target_link_libraries(FastLED INTERFACE

There is some more detail on this here https://datasheets.raspberrypi.com/pico/raspberry-pi-pico-c-sdk.pdf

As Peter says, we do have some better support for building static libraries in SDK1.5.0, but the cases you'd want to are pretty rare.

peterharperuk commented 1 year ago

If he doesn't use a static library, every source file in the sdk will have compat.hpp included at the top?

kilograham commented 1 year ago

oh - i guess i should read more closely - in all my years, i don't think i've ever noticed -include

anyhoo, yes you could therefore use _headers versions for the target_link_libraries

Alternatively you could stick with INTERFACE, and do

set_source_files_properties(
   FastLED/src/colorpalettes.cpp
    FastLED/src/colorutils.cpp
    FastLED/src/FastLED.cpp
    FastLED/src/hsv2rgb.cpp
    FastLED/src/platforms.cpp
    PROPERTIES
   COMPILE_FLAGS "-include ${CMAKE_CURRENT_SOURCE_DIR}/compat.hpp")

to just affect those files

cinderblock commented 1 year ago

Thank you all so much for the input! I have a couple things to try and these look quite promising! 😀

The _headers option for target_link_libraries seems like it is in line with my intent - however I'm not finding a clear example on how to use that...

While I'm here, some questions I can answer:

The INTERFACE option (to add_library and target_link_libraries) is one I'm certainly confused about. CMake's documentation seems to assume I know more about the rest of how CMake works and I find it rather unclear. I think the intent is, as suggested, to just use the "interface" (aka header files) from those dependencies since the sources will be linked in at a higher level. But other include errors cropped up making me think I was using it wrong. I was thinking of building a "STATIC" library since I've created intermediate .a files in other build chains in the past. Maybe CMake's expectations here don't quite line up with mine - based on comments here, "STATIC" is only for when your final output is a statically linkable library. I appreciate the clarification on this.

As for compat.hpp, that is my own creation. The Pico support for FastLED seems to assume its in an Arduino environment where those functions are declared and defined. And since the Arduino environment injects the #include "Arduino.h" above source files, FastLED expects somethings to have been declared "globally", so I created that tiny compatibility shim and need it included before their sources.

peterharperuk commented 1 year ago

I think I prefer the set_source_files_properties method to avoid using a static library. Static libraries can quickly cause weird build issues without you realising (in my experience).

I agree that the CMake documentation about INTERFACE libraries is a bit confusing. Everything in the sdk is an INTERFACE library. I just think of them as instructions for building a final executable. So source files, compile definitions, include folders and other library dependencies etc are added to the the final executable. The upshot is when you compile your program, the whole sdk gets built. This can be a bit wasteful and it needs some care, e.g. to avoid header file name clashes but it's also quite powerful allowing you to configure the whole sdk how you want just by adding the right "link" libraries dependencies to your executable.

cinderblock commented 1 year ago

I've been playing with this a bunch. I've also updated to SDK 1.5.0 (am on master actually. maybe I shouldn't)

The main thing that got me working was the set_source_files_properties(...) option. This correctly applied those compiler args only to the source files that needed it. Thank you all very much for that suggestion.

Moving onto the INTERFACE...

I found it builds mostly fine (more on this later) with or without STATIC, PRIVATE, or PUBLIC. I can make the FastLED library STATIC and it compiles and links as well. (I assumed STATIC would make a libFastLED.a as an intermediate file but that seems to be happening in my build logs no matter what I do).

However I cannot get INTERFACE to work in any way as you suggest:

Unless you deliberately wanted to use a static lib (which from your description, you don't), you should use

add_library(FastLED INTERFACE

and

target_link_libraries(FastLED INTERFACE
[cmake] CMake Error at lib/FastLED/CMakeLists.txt:3 (add_library):
[cmake]   add_library INTERFACE library requires no source arguments.

How do I define the sources of the library if not like this???

There is some more detail on this here https://datasheets.raspberrypi.com/pico/raspberry-pi-pico-c-sdk.pdf

I see much talk of the SDK being an "INTERFACE" library, but I guess I still don't understand what that means. The closest example in the pdf I saw to using INTERFACE with add_library is in Appendix B but I'm left with more questions than answers reading that.

Now, I've noticed something, that may be obvious to you all, likely caused by my misunderstanding of INTERFACE, but pico sources are getting built twice - in the FastLED library and for the main target.

To boot, while I can call some FastLED functions (FastLED.show()), if I include the required FastLED.addleds<NEOPIXEL, 60>(...) I get a confusing linker error: 

[build] /tmp/cczzgpwT.s: Assembler messages:
[build] /tmp/cczzgpwT.s:812: Error: invalid offset, value too big (0x00000454)

The only place I've seen this is in #97 but I'm not using LTO... Maybe this is a symptom of building and linking some pico functions twice? And the linker is trying to reconcile the duplicates which breaks jumps because they're now too far??

Thanks for all the help. Please forgive any typos or confusing messages. It's late. Time for sleep. 🛌💤

peterharperuk commented 1 year ago

How do I define the sources of the library if not like this???

In the version of cmake we're using, for an interface you have to add the sources in a separate line using target_sources, e.g. target_sources(NAMEOFTARGET INTERFACE ${CMAKE_CURRENT_LIST_DIR}/source.c etc) The gotcha is you have to include the full path of the source file.

Not really sure about your error - it looks like a build issue? Check what command is being called by passing VERBOSE=1 to make,

cinderblock commented 1 year ago

What do you mean by "version of cmake we're using"? Is this just fundamentally different in older versions?

Thank you for the suggestion. Curiously, I found that that the ${CMAKE_CURRENT_LIST_DIR}/ prefix in target_sources was not required. (I've tried with an without throughout the following as well).

I've got everything as INTERFACE now. I think this is making more sense.

However now I cannot get my -include option in on just the source files that need it.

set_source_files_properties(
    ${CMAKE_CURRENT_SOURCE_DIR}/FastLED/src/colorpalettes.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/FastLED/src/colorutils.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/FastLED/src/FastLED.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/FastLED/src/hsv2rgb.cpp
    ${CMAKE_CURRENT_SOURCE_DIR}/FastLED/src/platforms.cpp
    PROPERTIES
    COMPILE_OPTIONS "-include compat.hpp")

I've tried CMAKE_CURRENT_LIST_DIR instead of CMAKE_CURRENT_SOURCE_DIR. I've tried without either. I've tried manually writing out the path relative to different levels of my various CMakeLists.txt files and build directories.

I've tried a bunch of other options too:

I found this one kind of works, but applies to all source files, which causes other issues.

target_compile_options(FastLED INTERFACE $<$<COMPILE_LANGUAGE:CXX>:-include compat.hpp>)

So I'm trying to write a generator expression to filter for only source files in a certain path, but I haven't gotten quite to that working...

target_compile_options(FastLED INTERFACE $<$<AND:$<COMPILE_LANGUAGE:CXX>,$<...>>:-include compat.hpp>)

Is there some other (easier) way to achieve the behavior (of -include) that I want?

cinderblock commented 1 year ago

Is there some other (easier) way to achieve the behavior (of -include) that I want?

I was thinking about this a bunch and think I found an easier way.

I created a new FastLED.cpp:

#include "compat.hpp"

#include "FastLED/src/colorpalettes.cpp"
#include "FastLED/src/colorutils.cpp"
#include "FastLED/src/FastLED.cpp"
#include "FastLED/src/hsv2rgb.cpp"
#include "FastLED/src/platforms.cpp"

I then tell CMake that is the only source file for this INTERFACE library and we're (basically) off to the races! -include not required!

The problem remains with this "Assembler messages" when compiling main.cpp:

/usr/bin/arm-none-eabi-g++ -DLIB_CMSIS_CORE=1 -DLIB_PICO_BIT_OPS=1 -DLIB_PICO_BIT_OPS_PICO=1 -DLIB_PICO_DIVIDER=1 -DLIB_PICO_DIVIDER_HARDWARE=1 -DLIB_PICO_DOUBLE=1 -DLIB_PICO_DOUBLE_PICO=1 -DLIB_PICO_FLOAT=1 -DLIB_PICO_FLOAT_PICO=1 -DLIB_PICO_INT64_OPS=1 -DLIB_PICO_INT64_OPS_PICO=1 -DLIB_PICO_MALLOC=1 -DLIB_PICO_MEM_OPS=1 -DLIB_PICO_MEM_OPS_PICO=1 -DLIB_PICO_PLATFORM=1 -DLIB_PICO_PRINTF=1 -DLIB_PICO_PRINTF_PICO=1 -DLIB_PICO_RUNTIME=1 -DLIB_PICO_STANDARD_LINK=1 -DLIB_PICO_STDIO=1 -DLIB_PICO_STDIO_UART=1 -DLIB_PICO_STDLIB=1 -DLIB_PICO_SYNC=1 -DLIB_PICO_SYNC_CRITICAL_SECTION=1 -DLIB_PICO_SYNC_MUTEX=1 -DLIB_PICO_SYNC_SEM=1 -DLIB_PICO_TIME=1 -DLIB_PICO_UTIL=1 -DPICO_BOARD=\"pico\" -DPICO_BUILD=1 -DPICO_CMAKE_BUILD_TYPE=\"Release\" -DPICO_COPY_TO_RAM=0 -DPICO_CXX_ENABLE_EXCEPTIONS=0 -DPICO_NO_FLASH=0 -DPICO_NO_HARDWARE=0 -DPICO_ON_DEVICE=1 -DPICO_TARGET_NAME=\"my_executable\" -DPICO_USE_BLOCKED_RAM=0 -I../lib/SDK/src/common/pico_stdlib/include -I../lib/SDK/src/rp2_common/hardware_gpio/include -I../lib/SDK/src/common/pico_base/include -Igenerated/pico_base -I../lib/SDK/src/boards/include -I../lib/SDK/src/rp2_common/pico_platform/include -I../lib/SDK/src/rp2040/hardware_regs/include -I../lib/SDK/src/rp2_common/hardware_base/include -I../lib/SDK/src/rp2040/hardware_structs/include -I../lib/SDK/src/rp2_common/hardware_claim/include -I../lib/SDK/src/rp2_common/hardware_sync/include -I../lib/SDK/src/rp2_common/hardware_irq/include -I../lib/SDK/src/common/pico_sync/include -I../lib/SDK/src/common/pico_time/include -I../lib/SDK/src/rp2_common/hardware_timer/include -I../lib/SDK/src/common/pico_util/include -I../lib/SDK/src/rp2_common/hardware_uart/include -I../lib/SDK/src/rp2_common/hardware_resets/include -I../lib/SDK/src/rp2_common/hardware_clocks/include -I../lib/SDK/src/rp2_common/hardware_pll/include -I../lib/SDK/src/rp2_common/hardware_vreg/include -I../lib/SDK/src/rp2_common/hardware_watchdog/include -I../lib/SDK/src/rp2_common/hardware_xosc/include -I../lib/SDK/src/rp2_common/hardware_divider/include -I../lib/SDK/src/rp2_common/pico_runtime/include -I../lib/SDK/src/rp2_common/pico_printf/include -I../lib/SDK/src/common/pico_bit_ops/include -I../lib/SDK/src/common/pico_divider/include -I../lib/SDK/src/rp2_common/pico_double/include -I../lib/SDK/src/rp2_common/pico_float/include -I../lib/SDK/src/rp2_common/pico_malloc/include -I../lib/SDK/src/rp2_common/pico_bootrom/include -I../lib/SDK/src/common/pico_binary_info/include -I../lib/SDK/src/rp2_common/pico_stdio/include -I../lib/SDK/src/rp2_common/pico_stdio_uart/include -I../lib/SDK/src/rp2_common/pico_int64_ops/include -I../lib/SDK/src/rp2_common/pico_mem_ops/include -I../lib/SDK/src/rp2_common/boot_stage2/include -I../lib/SDK/src/rp2_common/hardware_spi/include -I../lib/SDK/src/rp2_common/hardware_i2c/include -I../lib/FastLED/compat -I../lib/FastLED/FastLED/src -I../lib/SDK/src/rp2_common/hardware_dma/include -I../lib/SDK/src/rp2_common/hardware_pio/include -I../lib/SDK/src/rp2_common/cmsis/stub/CMSIS/Core/Include -I../lib/SDK/src/rp2_common/cmsis/stub/CMSIS/Device/RaspberryPi/RP2040/Include -mcpu=cortex-m0plus -mthumb -O3 -DNDEBUG -ffunction-sections -fdata-sections -fno-exceptions -fno-unwind-tables -fno-rtti -fno-use-cxa-atexit -MD -MT CMakeFiles/my_executable.dir/src/main.cpp.obj -MF CMakeFiles/my_executable.dir/src/main.cpp.obj.d -o CMakeFiles/my_executable.dir/src/main.cpp.obj -c ../src/main.cpp
/tmp/ccasthV7.s: Assembler messages:
/tmp/ccasthV7.s:812: Error: invalid offset, value too big (0x00000454)

I suspect this is a FastLED bug at this point. I'll probably start a new thread about getting this to work there. Until then I'm going to post my findings here. Any pointers would be greatly appreciated :)

Manually building with --save-temps I can capture the intermediate assembly file that is generating the issues:

; ...
    .syntax divided
@ 279 "../lib/FastLED/compat/../FastLED/src/platforms/arm/rp2040/../common/m0clockless.h" 1
      loadleds3 ip, r4, #1 ,r3;  loaddither7 r4, r5, r6, #1;  dither5 r4, r5;  scale4 r4, r6, #12, r3;  adjdither7 r6,r5,#1,#4,r3;  swapbbn1 r2, r4;
@ 0 "" 2
    .thumb
    .syntax unified
    ldr r6, .L131                     ; <------- Line 812: Error: invalid offset, value too big
    movs    r0, r2
    str r6, [sp, #8]
    ldr r6, [r6]
    mov r2, ip
    str r6, [sp]
    b   .L124
.L130:
    cmp r1, #0
    bne .LCB824
    b   .L129   @long jump
.LCB824:
.L124:
    mov r10, r2
    mov r7, r8
    ldr r2, [sp, #4]
    mov r6, r9
    .syntax divided
; ...

Note:

/usr/bin/arm-none-eabi-g++ --version
arm-none-eabi-g++ (15:8-2019-q3-1+b1) 8.3.1 20190703 (release) [gcc-8-branch revision 273027]
peterharperuk commented 1 year ago

What do you mean by "version of cmake we're using"?

Later versions of cmake allow you to add sources on the same line. I think you should continue this discussion on the forum.