Skip to content

Assembling compile_time_choice.S does not respect PICO_BOARD_HEADER_DIRS #2114

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

Open
Memotech-Bill opened this issue Nov 28, 2024 · 13 comments
Open
Assignees
Labels
Milestone

Comments

@Memotech-Bill
Copy link

A board header file may include another (secondary) header file.

Providing the secondary header file is in pico-sdk/src/boards/include, then everything works.

However if the secondary header file is in a user defined folder, which is specified in PICO_BOARD_HEADER_DIRS, then the compilation of compile_time_choice.S fails:

[  1%] Building ASM object pico-sdk/src/rp2350/boot_stage2/CMakeFiles/bs2_default.dir/compile_time_choice.S.o
cd /home/pi/pico/PicoBB/console/pico_w/build/pico-sdk/src/rp2350/boot_stage2 && /usr/bin/arm-none-eabi-gcc -DLIB_BOOT_STAGE2_HEADERS=1 -DPICO_32BIT=1 -DPICO_BOARD=\"vgaboard_cut_2w\" -DPICO_BUILD=1 -DPICO_FLASH_SIZE_BYTES="(2 * 1024 * 1024)" -DPICO_NO_HARDWARE=0 -DPICO_ON_DEVICE=1 -DPICO_RP2350=1 -I/home/pi/pico/pico-sdk/src/rp2350/boot_stage2/asminclude -isystem /home/pi/pico/pico-sdk/src/rp2350/hardware_regs/include -isystem /home/pi/pico/pico-sdk/src/rp2_common/hardware_base/include -isystem /home/pi/pico/pico-sdk/src/common/pico_base_headers/include -isystem /home/pi/pico/PicoBB/console/pico_w/build/generated/pico_base -isystem /home/pi/pico/pico-sdk/src/boards/include -isystem /home/pi/pico/pico-sdk/src/rp2350/pico_platform/include -isystem /home/pi/pico/pico-sdk/src/rp2_common/pico_platform_compiler/include -isystem /home/pi/pico/pico-sdk/src/rp2_common/pico_platform_panic/include -isystem /home/pi/pico/pico-sdk/src/rp2_common/pico_platform_sections/include -isystem /home/pi/pico/pico-sdk/src/rp2_common/hardware_dcp/include -isystem /home/pi/pico/pico-sdk/src/rp2350/hardware_structs/include -isystem /home/pi/pico/pico-sdk/src/rp2_common/hardware_rcp/include -isystem /home/pi/pico/pico-sdk/src/rp2350/boot_stage2/include -mcpu=cortex-m33 -mthumb -march=armv8-m.main+fp+dsp -mfloat-abi=softfp -mcmse -g -O3 -DNDEBUG -o CMakeFiles/bs2_default.dir/compile_time_choice.S.o   -c /home/pi/pico/pico-sdk/src/rp2350/boot_stage2/compile_time_choice.S
In file included from /home/pi/pico/PicoBB/console/pico_w/build/generated/pico_base/pico/config_autogen.h:7,
                 from /home/pi/pico/pico-sdk/src/common/pico_base_headers/include/pico/config.h:19,
                 from /home/pi/pico/pico-sdk/src/rp2350/boot_stage2/include/boot_stage2/config.h:12,
                 from /home/pi/pico/pico-sdk/src/rp2350/boot_stage2/compile_time_choice.S:11:
/home/pi/pico/PicoBB/boards/vgaboard_cut_2w.h:91:10: fatal error: boards/pico2+w.h: No such file or directory
 #include "boards/pico2+w.h"
          ^~~~~~~~~~~~~~~~~~
compilation terminated.

My user defined directory:

pi@raspberrypi:~/pico/PicoBB/console/pico_w $ ls ../../boards/
pico2+w.h  vgaboard_cut_2w.h   vgaboard_cut.h    vgaboard_sd.h    vgaboard_serial.h
pico+w.h   vgaboard_cut_2w.h~  vgaboard_cut_w.h  vgaboard_sd_w.h  vgaboard_serial_w.h

config_autogen.h

// AUTOGENERATED FROM PICO_CONFIG_HEADER_FILES and then PICO_<PLATFORM>_CONFIG_HEADER_FILES
// DO NOT EDIT!


// based on PICO_CONFIG_HEADER_FILES:

#include "/home/pi/pico/PicoBB/boards/vgaboard_cut_2w.h"
#include "/home/pi/pico/pico-sdk/src/rp2_common/cmsis/include/cmsis/rename_exceptions.h"

// based on PICO_RP2350_ARM_S_CONFIG_HEADER_FILES:
@lurch
Copy link
Contributor

lurch commented Nov 29, 2024

@will-v-pi Would a similar thing also affect #2113 ?

@Memotech-Bill
Copy link
Author

@will-v-pi Would a similar thing also affect #2113 ?

I don't think so. If it did I would have obtained a failure in CMake before getting to compiling compile_time_choice.S.

@lurch
Copy link
Contributor

lurch commented Nov 29, 2024

I assumed that you weren't testing #2113 with custom header files in a user-defined PICO_BOARD_HEADER_DIRS location? 🤷

Perhaps I should have made the question in my previous comment more explicit: if there's an #include of a custom header file in a user-defined PICO_BOARD_HEADER_DIRS location, and that custom header file contains // pico_cmake_set declaations, will those declarations get included or ignored by the code in #2113 ?

@Memotech-Bill
Copy link
Author

I assumed that you weren't testing #2113 with custom header files in a user-defined PICO_BOARD_HEADER_DIRS location? 🤷

For my first test, the primary board header file was in a user defined location, but the secondary board header file was pico2_w.h in the standard location. That all worked, hence my report of success.

Perhaps I should have made the question in my previous comment more explicit: if there's an #include of a custom header file in a user-defined PICO_BOARD_HEADER_DIRS location, and that custom header file contains // pico_cmake_set declaations, will those declarations get included or ignored by the code in #2113 ?

I then went on to do a test with both primary and secondary board header files in a user defined location. I could see from the CMake output that this had correctly picked up the pico_cmake_set declarations from the secondary header file but then failed on compiling compile_time_choice.

@will-v-pi
Copy link
Contributor

if there's an #include of a custom header file in a user-defined PICO_BOARD_HEADER_DIRS location, and that custom header file contains // pico_cmake_set declaations, will those declarations get included or ignored by the code in #2113 ?

They will still be included by #2113 - it searches all of PICO_BOARD_HEADER_DIRS in the same way that the SDK currently does when searching for included board files.

@lurch lurch added the build label Nov 29, 2024
@kilograham kilograham self-assigned this Jan 7, 2025
@kilograham kilograham added this to the 2.1.1 milestone Jan 31, 2025
@kilograham
Copy link
Contributor

I think you should just use #include "pico2+w.h"

@kilograham kilograham modified the milestones: 2.1.1, 2.2.0 Feb 3, 2025
@Memotech-Bill
Copy link
Author

Interesting.

I had used the boards/ prefix because the stock vgaboard.h file has:

#include "boards/pico.h"

A quick test suggests that omitting the boards/ prefix works whether the nested include file is in either the SDK default folder or the user defined one.

My test is not definitive because I have re-arranged my code to avoid the need of having nested includes, and quickly hacking one back in results in a compilation failure for other causes. But I don't get any "No such file or directory" errors for the nested include file.

@Memotech-Bill
Copy link
Author

Memotech-Bill commented Feb 3, 2025

A quick test suggests that omitting the boards/ prefix works whether the nested include file is in either the SDK default folder or the user defined one.

My test was too quick. If the nested include file is in the SDK default directory, then you do have to prefix the file name by the boards/ path, whereas if in the user defined folder, the prefix should be omitted.

It would be nice if the behaviour was consistent between the two locations.

@lurch
Copy link
Contributor

lurch commented Feb 20, 2025

If the nested include file is in the SDK default directory, then you do have to prefix the file name by the boards/ path, whereas if in the user defined folder, the prefix should be omitted.

IMHO that makes sense.

It would be nice if the behaviour was consistent between the two locations.

Do you get the consistency you're after if you create a boards directory inside your user-defined folder?

@Memotech-Bill
Copy link
Author

No, because cmake does not look in $(PICO_BOARD_HEADER_DIRS)/boards

Target board (PICO_BOARD) is 'test'.
CMake Error at /home/pi/pico/pico-sdk/cmake/generic_board.cmake:40 (message):
  Unable to find definition of board 'test' (specified by PICO_BOARD):

     Looked for test.h in /home/pi/pico/PicoBB/src/pico/../../boards, /home/pi/pico/pico-sdk/cmake/../src/boards/include/boards (additional paths specified by PICO_BOARD_HEADER_DIRS)
     Looked for test.cmake in /home/pi/pico/pico-sdk/cmake/../src/boards (additional paths specified by PICO_BOARD_CMAKE_DIRS)
Call Stack (most recent call first):
  /home/pi/pico/pico-sdk/cmake/pico_pre_load_platform.cmake:70 (include)
  /home/pi/pico/pico-sdk/pico_sdk_init.cmake:88 (include)
  /home/pi/pico/pico-sdk/external/pico_sdk_import.cmake:121 (include)
  CMakeLists.txt:19 (include)

My CMakeLists.txt file contains the line:

set(PICO_BOARD_HEADER_DIRS ${CMAKE_CURRENT_LIST_DIR}/../../boards)

And the files are located at:

pi@raspberrypi:~/pico/PicoBB $ find -name CMakeLists.txt -print
./src/pico/CMakeLists.txt
pi@raspberrypi:~/pico/PicoBB $ find -name test.h -print
./boards/boards/test.h

@Memotech-Bill
Copy link
Author

The way to make the custom board files consistent with the SDK, without breaking any existing builds would be:

  • cmake searches $(PICO_BOARD_HEADER_DIRS)/include/boards and then $(PICO_BOARD_HEADER_DIRS) for board header files.
  • The C include folders includes $(PICO_BOARD_HEADER_DIRS)/include and $(PICO_BOARD_HEADER_DIRS).

@lurch
Copy link
Contributor

lurch commented Feb 21, 2025

If you don't mind me asking, why is this "consistency" so important to you? Perhaps the different include-styles of:

#include "boards/pico.h"
#include "pico2+w.h"

makes it more obvious that pico.h is getting included from the SDK, but pico2+w.h is getting included from your user-defined directory? 🤷‍♂

@Memotech-Bill
Copy link
Author

I suppose just because I got bitten by it not being obvious that I needed to use the different style when including files from my user defined directory.

If it was not obvious to me, how many others will encounter the same issue?

But on further thought, my proposal of having boards/include/boards, while being consistent, is not obvious either.

The hack that would probably work for most people who make the same mistake as I did would be to have $(PICO_BOARD_HEADER_DIRS)/.. as an include folder.

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

No branches or pull requests

4 participants