cpu/nrf5x: Pull Vendor Header Files from External Repository#21800
cpu/nrf5x: Pull Vendor Header Files from External Repository#21800maribu merged 2 commits intoRIOT-OS:masterfrom
Conversation
crasbe
added
Type: enhancement
github-actions
bot
added
Platform: ARM
Nice :) |
github-actions
bot
added
the
Area: CI
crasbe
removed
the
Community: help wanted
crasbe
force-pushed
the
pr/nrf52_vendor
branch
from
06b948e to
779d095
Compare
crasbe
force-pushed
the
pr/nrf52_vendor
branch
from
779d095 to
a0a6b7f
Compare
github-actions
bot
removed
the
Area: CI
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
crasbe
force-pushed
the
pr/nrf52_vendor
branch
from
a0a6b7f to
d7c17f0
Compare
|
So I had to change basically two things: the patching of the vendor files worked only once, so switching from nRF52 to nRF53 caused an error related to the unpatched header. This did not occur in CI because CI always starts with a clean slate. The second thing is that apparently there are different nRF51 variants, nRF51x22xxaa and nRF51x22xxab. Therefore I just added a wildcard to after the |
crasbe
force-pushed
the
pr/nrf52_vendor
branch
from
dd9e8dd to
e5b1f31
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
That wasn't enough because we're not pulling all the nRF52xxx files at once but only the required ones. Changing the subdirectories to |
crasbe
force-pushed
the
pr/nrf52_vendor
branch
from
2224d07 to
4752d2e
Compare
|
Perhaps I shouldn't have squashed it already... sorry :/ |
|
No, please squash right away after the ACK, unless the change is significant |
|
Hooray! 🎉 |
|
I just tried building the RIOT/build/nrf5x_nrfx_mdk/nrf52/drivers/nrfx_common.h:46:10: fatal error: nrf_mem.h: No such file or directory
46 | #include <nrf_mem.h>
| ^~~~~~~~~~~
compilation terminated.I guess it's somehow related to this PR? |
|
I can reproduce it locally, both with The CI has no issue, though: https://ci.riot-os.org/results/a38f322d29e149bfb91e9b92540e497e/output/builds/examples/networking/gnrc/gnrc_networking/nrf52dk:gnu.txt |
diff --git a/cpu/nrf5x_common/Makefile.include b/cpu/nrf5x_common/Makefile.include
index cef6356d92..038c7701c3 100644
--- a/cpu/nrf5x_common/Makefile.include
+++ b/cpu/nrf5x_common/Makefile.include
@@ -19,7 +19,7 @@ ifneq (,$(filter nrf53%,$(CPU_MODEL)))
else
NRF5XFAM_INCLUDE_FILE = $(NRF5XFAM_INCLUDE_DIR)/vendor/$(CPU_FAM).h
endif
-INCLUDES += -I$(NRF5XFAM_INCLUDE_DIR)
+INCLUDES += -I$(NRF5XFAM_INCLUDE_DIR) -I$(NRF5XFAM_INCLUDE_DIR)/vendor
# Fetch all nRF52 vendor headers using the package mechanism. This rule is called all
# the time to ensure it's correctly updated when versions in the packages arefixes the issue for me. But I think we probably should first look into reproducing the issue in the CI to prevent this from popping up again before fixing the issue. |
|
I know why that happens locally and not with the CI... The CI uses the Rust Git-Cache while you probably don't use it locally. The "old" version of the git cache as well as the non-cached versions don't do sparse checkouts. Therefore it clones the full |
cpu/nrf5x_common: fix vendor header regression from #21800
5 participants
Contribution description
As discussed in #21739, it would be nice to exclude the Nordic nRF5x vendor header files from RIOT, as these are big and updating them means multi-thousand line commits and pull requests.
Current issues:
git-cache-rs, the build system will not perform a sparse checkout and will download the full 230-ish MB ofnrfx, this is not ideal.The package is currently namedFixed: now using the same mechanism thatanrfx_mdkbecause it has to be built beforempaland-printf, and inconveniently N comes after M in the alphabet... This has never been a problem before becausecmsisstarts with C andesp32-sdkwith E, which are both waaaay before M.stm32uses.Ideally I would like to avoid havingnrfxandnrfx_mdkas essentially the same packages, however I'm not sure how to handle the case when the fullnrfxis used.Status:
Only nRF52832 and nRF52840 work, but migrating everything else is trivial (I just have to add the PKG_SPARSE_PATHs).Testing procedure
Make sure all your applications still work.
Issues/PRs references
Came up in #21739.