Buildsystem: Automatically add Common Board Definitions when USEMODULE is set#21299
Closed
crasbe wants to merge 5 commits intoRIOT-OS:masterfrom
Closed
Buildsystem: Automatically add Common Board Definitions when USEMODULE is set#21299crasbe wants to merge 5 commits intoRIOT-OS:masterfrom
USEMODULE is set#21299crasbe wants to merge 5 commits intoRIOT-OS:masterfrom
Conversation
crasbe
added
State: WIP
github-actions
bot
added
Area: doc
crasbe
added
the
Community: help wanted
Murdock results❌ FAILED d60dcb4 fixup! fixup! fixup! boards/*: adapt boards to new common boards Build failures (21)
Artifacts |
crasbe
force-pushed
the
pr/boards_common
branch
2 times, most recently
from
8947c43 to
81f7aa2
Compare
mguetschow
reviewed
Mar 17, 2025
Makefile.include
Outdated
Comment on lines
+611
to
+612
| COMMON_BOARD_DIRS := $(subst _common_,/common/,$(COMMON_BOARD_MODULES)) | ||
| COMMON_BOARD_DIRS := $(addprefix $(RIOTBASE)/,$(COMMON_BOARD_DIRS)) |
Contributor
There was a problem hiding this comment.
This introduces a 1-to-1 relationship between the module name and it's folder structure. While I do think that makes sense, I think this is new to RIOT.
Could we mimic the way sys/Makefile* are organized and instead add that to dirs in application.inc.mk?
crasbe
force-pushed
the
pr/boards_common
branch
from
81f7aa2 to
7fd7d2e
Compare
Contributor
Author
|
This is a much bigger task than I imagined... the fixup comment is a start for the new approach, but there is still a lot to do: I'm not sure how to proceed with the Lines 3 to 5 in 04a1698 |
crasbe
force-pushed
the
pr/boards_common
branch
from
31cdbcc to
250bef0
Compare
crasbe
force-pushed
the
pr/boards_common
branch
2 times, most recently
from
338944c to
2deeb69
Compare
crasbe
removed
the
State: waiting for other PR
crasbe
force-pushed
the
pr/boards_common
branch
from
3880883 to
ed8dcba
Compare
Contributor
Author
|
Okay the Microbit code is a bit weird, apparently the |
crasbe
force-pushed
the
pr/boards_common
branch
from
ed8dcba to
5738655
Compare
Contributor
Author
|
One could argue that this PR would be a good opportunity to add the That would really clean up the board definition Makefiles IMO and you would only have to add the |
crasbe
force-pushed
the
pr/boards_common
branch
from
5738655 to
d60dcb4
Compare
Contributor
Author
|
I think this PR has to be split up into separate PRs:
Otherwise this will be a cat-and-mouse game with CI with a lot of potential to break things 😅 Tomorrow I'll split this up and possibly create a tracking issue with groups that have to be adapted. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
Many boards in RIOT (such as the
feather-nrf52840,feather-m0, ...) use common board definitions for things like the bootloader. Back when this was implemented in #8058, the approach was to include the directory by adding the path to theDIRSvariable and setting theUSEMODULEin one of the common folders.This is not really elegant and the
Makefilein theboards/common/...subfolder didn't even get executed.You can find more information about this in #21298.
The PR is based on #21281 and since it is not currently merged, I added the changes in squashed-form. That is obviously to be deleted once #21281 is merged.This is meant as a Request for Comments.If this is valid solution, it would be possible to adapt it to the CPU common folders as well, but probably not in this PR.
Testing procedure
Make sure the boards still compile and the common folder is still included.
The only adapted boards are currently the two
feather-nrf52840*and theseeedstudio-xiao-nrf52840.`tests/sys/shell` on `feather-nrf52840` with #21281:
`tests/sys/shell` on `feather-nrf52840` with this PR:
Boards that are not yet adapted, such as the
feather-m0, will include the common files twice. I don't think that that would have any adverse effects, but it's obviously not desireable.`tests/sys/shell` on `feather-m0` with master
`tests/sys/shell` on `feather-m0` with this PR:
Issues/PRs references
Fixes #21298.