Parse included board files for pico_cmake_set declarations #2968

Merged
will-v-pi merged 4 commits from cmake-set-includes into develop 2026-05-28 09:55:12 -05:00
will-v-pi commented 2026-05-26 04:18:33 -05:00 (Migrated from github.com)

Currently if you have a #include within a board header file (eg in vgaboard.h) any pico_cmake_set and pico_cmake_set_default declarations in the included file aren't parsed (#2112)

This fixes #2112 by parsing those files for these comments. It only parses includes which start with boards/, as it needs to extract the .h file directly for use with pico_find_in_paths.

The inclusion order matches what would be expected (I think) - ie:

// pico_cmake_set PICO_PLATFORM = rp2350

#include "boards/pico.h"

gives PICO_PLATFORM = rp2040 from pico.h,

#include "boards/pico.h"

// pico_cmake_set PICO_PLATFORM = rp2350

gives PICO_PLATFORM = rp2350,

// pico_cmake_set_default PICO_FLASH_SIZE_BYTES = (8 * 1024 * 1024)
#ifndef PICO_FLASH_SIZE_BYTES
#define PICO_FLASH_SIZE_BYTES (8 * 1024 * 1024)
#endif

#include "boards/pico.h"

gives PICO_FLASH_SIZE_BYTES = (8 * 1024 * 1024) (matching the pre-processor defines), and

#include "boards/pico.h"

// pico_cmake_set_default PICO_FLASH_SIZE_BYTES = (8 * 1024 * 1024)
#ifndef PICO_FLASH_SIZE_BYTES
#define PICO_FLASH_SIZE_BYTES (8 * 1024 * 1024)
#endif

gives PICO_FLASH_SIZE_BYTES = (2 * 1024 * 1024) from pico.h (again matching the pre-processor defines)

This is a duplicate of #2113, which I accidentally closed

Currently if you have a `#include` within a board header file (eg in [vgaboard.h](https://github.com/raspberrypi/pico-sdk/blob/master/src/boards/include/boards/vgaboard.h)) any `pico_cmake_set` and `pico_cmake_set_default` declarations in the included file aren't parsed (#2112) This fixes #2112 by parsing those files for these comments. It only parses includes which start with `boards/`, as it needs to extract the `.h` file directly for use with `pico_find_in_paths`. The inclusion order matches what would be expected (I think) - ie: ```c // pico_cmake_set PICO_PLATFORM = rp2350 #include "boards/pico.h" ``` gives `PICO_PLATFORM = rp2040` from `pico.h`, ```c #include "boards/pico.h" // pico_cmake_set PICO_PLATFORM = rp2350 ``` gives `PICO_PLATFORM = rp2350`, ```c // pico_cmake_set_default PICO_FLASH_SIZE_BYTES = (8 * 1024 * 1024) #ifndef PICO_FLASH_SIZE_BYTES #define PICO_FLASH_SIZE_BYTES (8 * 1024 * 1024) #endif #include "boards/pico.h" ``` gives `PICO_FLASH_SIZE_BYTES = (8 * 1024 * 1024)` (matching the pre-processor defines), and ```c #include "boards/pico.h" // pico_cmake_set_default PICO_FLASH_SIZE_BYTES = (8 * 1024 * 1024) #ifndef PICO_FLASH_SIZE_BYTES #define PICO_FLASH_SIZE_BYTES (8 * 1024 * 1024) #endif ``` gives `PICO_FLASH_SIZE_BYTES = (2 * 1024 * 1024)` from `pico.h` (again matching the pre-processor defines) This is a duplicate of #2113, which I accidentally closed
kilograham (Migrated from github.com) requested changes 2026-05-26 16:37:32 -05:00
kilograham (Migrated from github.com) left a comment

seems like this could be an elseif

seems like this could be an elseif
@ -41,6 +42,13 @@ if (EXISTS ${PICO_BOARD_HEADER_FILE})
else()
kilograham (Migrated from github.com) commented 2026-05-26 16:37:23 -05:00

seems like this could be an elseif - not i don't particularly care, but since I'm gonna suggest you also prevent foot-gunning on recursive includes (which might otherwise be fixed by include guards in the headers themselves)

seems like this could be an `elseif` - not i don't particularly care, but since I'm gonna suggest you also prevent foot-gunning on recursive includes (which might otherwise be fixed by include guards in the headers themselves)
lurch (Migrated from github.com) reviewed 2026-05-27 05:02:40 -05:00
lurch (Migrated from github.com) commented 2026-05-27 05:02:40 -05:00

I'm no CMake expert, but why is this regex doing [ \t\]* when [ \t]* is being used on lines 35 and 38?

I'm no CMake expert, but why is this regex doing `[ \t\]*` when `[ \t]*` is being used on lines 35 and 38?
will-v-pi (Migrated from github.com) reviewed 2026-05-27 05:08:47 -05:00
will-v-pi (Migrated from github.com) commented 2026-05-27 05:08:47 -05:00

Because this PR was originally created before https://github.com/raspberrypi/pico-sdk/pull/2526 - I will remove the unnecessary escapes

Because this PR was originally created before https://github.com/raspberrypi/pico-sdk/pull/2526 - I will remove the unnecessary escapes
lurch (Migrated from github.com) reviewed 2026-05-27 06:25:26 -05:00
@ -14,6 +14,7 @@ pico_find_in_paths(PICO_BOARD_HEADER_FILE PICO_BOARD_HEADER_DIRS ${PICO_BOARD}.h
if (EXISTS ${PICO_BOARD_HEADER_FILE})
message("Using board configuration from ${PICO_BOARD_HEADER_FILE}")
list(APPEND PICO_CONFIG_HEADER_FILES ${PICO_BOARD_HEADER_FILE})
list(APPEND PICO_BOARD_INCLUDED_FILES ${PICO_BOARD_HEADER_FILE})
lurch (Migrated from github.com) commented 2026-05-27 06:25:26 -05:00

Should PICO_INCLUDED_BOARD_HEADER_FILE also be appended to PICO_CONFIG_HEADER_FILES? 🤔 And if so, could you just use PICO_CONFIG_HEADER_FILES instead of PICO_BOARD_INCLUDED_FILES ? (Apologies in advance if this is a stupid suggestion!)

Should `PICO_INCLUDED_BOARD_HEADER_FILE` also be appended to `PICO_CONFIG_HEADER_FILES`? :thinking: And if so, could you just use `PICO_CONFIG_HEADER_FILES` instead of `PICO_BOARD_INCLUDED_FILES` ? (Apologies in advance if this is a stupid suggestion!)
lurch commented 2026-05-27 06:26:16 -05:00 (Migrated from github.com)

Is a similar fix to this needed for the Bazel side of things? Or is this only needed for CMake?

Is a similar fix to this needed for the Bazel side of things? Or is this only needed for CMake?
will-v-pi (Migrated from github.com) reviewed 2026-05-27 06:30:42 -05:00
@ -14,6 +14,7 @@ pico_find_in_paths(PICO_BOARD_HEADER_FILE PICO_BOARD_HEADER_DIRS ${PICO_BOARD}.h
if (EXISTS ${PICO_BOARD_HEADER_FILE})
message("Using board configuration from ${PICO_BOARD_HEADER_FILE}")
list(APPEND PICO_CONFIG_HEADER_FILES ${PICO_BOARD_HEADER_FILE})
list(APPEND PICO_BOARD_INCLUDED_FILES ${PICO_BOARD_HEADER_FILE})
will-v-pi (Migrated from github.com) commented 2026-05-27 06:30:42 -05:00

No, it doesn't need to be added to PICO_CONFIG_HEADER_FILES (used to make config_autogen.h), as it's being included by the base board file already in the C/C++ build

No, it doesn't need to be added to `PICO_CONFIG_HEADER_FILES` (used to make `config_autogen.h`), as it's being included by the base board file already in the C/C++ build
will-v-pi commented 2026-05-27 06:35:04 -05:00 (Migrated from github.com)

Is a similar fix to this needed for the Bazel side of things? Or is this only needed for CMake?

Bazel handles boards differently to CMake, and doesn't use the pico_board_cmake_set/pico_board_cmake_set_default values, so this fix isn't needed there

> Is a similar fix to this needed for the Bazel side of things? Or is this only needed for CMake? Bazel handles boards differently to CMake, and doesn't use the `pico_board_cmake_set`/`pico_board_cmake_set_default` values, so this fix isn't needed there
kilograham (Migrated from github.com) approved these changes 2026-05-28 09:55:04 -05:00
Sign in to join this conversation.
No reviewers
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-raspberrypi/pico-sdk!2968
No description provided.