Skip to content
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

Parse included board files for pico_cmake_set declarations #2113

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

will-v-pi
Copy link
Contributor

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)

@lurch
Copy link
Contributor

lurch commented Nov 28, 2024

If the behaviour matches what the pre-processor does, then I think this sounds great! Well done @will-v-pi 😃

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

Successfully merging this pull request may close these issues.

2 participants