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

gap in support for deprecated Nordic board names #24158

Closed
pabigot opened this issue Apr 7, 2020 · 4 comments · Fixed by #24205
Closed

gap in support for deprecated Nordic board names #24158

pabigot opened this issue Apr 7, 2020 · 4 comments · Fixed by #24205
Assignees
Labels
area: Boards bug The issue is a bug, or the PR is fixing a bug platform: nRF Nordic nRFx priority: low Low impact/importance bug

Comments

@pabigot
Copy link
Collaborator

pabigot commented Apr 7, 2020

Something seems to be failing with the deprecated Nordic board names.

samples/subsys/shell/fs/CMakeLists.txt looks at the board name to determine whether the mpu.conf overlay is required. The file was updated to use the new board names, and the addition is not performed for the deprecated name.

tirzah[346]$ rm -rf build && west build -b nrf52840dk_nrf52840 samples/subsys/shell/fs >/dev/null && grep CONFIG_MPU_ALLOW build/zephyr/.config 
Including boilerplate (Zephyr base): /mnt/nordic/zp/zephyr/cmake/app/boilerplate.cmake
CONFIG_MPU_ALLOW_FLASH_WRITE=y

tirzah[347]$ rm -rf build && west build -b nrf52840_pca10056 samples/subsys/shell/fs >/dev/null && grep CONFIG_MPU_ALLOW build/zephyr/.config 
Including boilerplate (Zephyr base): /mnt/nordic/zp/zephyr/cmake/app/boilerplate.cmake
CMake Warning at /mnt/nordic/zp/zephyr/cmake/app/boilerplate.cmake:200 (message):
  Deprecated BOARD=nrf52840_pca10056 name specified, board automatically
  changed to: nrf52840dk_nrf52840.
Call Stack (most recent call first):
  /mnt/nordic/zp/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:24 (include)
  /mnt/nordic/zp/zephyr/share/zephyr-package/cmake/ZephyrConfig.cmake:35 (include_boilerplate)
  CMakeLists.txt:17 (find_package)


# CONFIG_MPU_ALLOW_FLASH_WRITE is not set

@pabigot pabigot added the bug The issue is a bug, or the PR is fixing a bug label Apr 7, 2020
@pabigot
Copy link
Collaborator Author

pabigot commented Apr 7, 2020

@carlescufi This is the underlying cause of #24145.

@carlescufi
Copy link
Member

carlescufi commented Apr 8, 2020

@tejlmand can you take a look please? If this is intended (i.e. some cases are not supported) then it is OK to close the issue.
@pabigot thanks for the analysis

@carlescufi carlescufi added area: Boards platform: nRF Nordic nRFx priority: low Low impact/importance bug labels Apr 8, 2020
@tejlmand
Copy link
Collaborator

tejlmand commented Apr 8, 2020

This happens because the sample CMakeLists.txt itself tests the board name BEFORE renaming is done.

Hence, it doesn't know anything about deprecated boards.
Secondly, it also happens before boilerplate is sourced, making things trickier.

Of course the easy fix is to just update to test both new and deprecated boards, but as this also affect aliases, it should be considered if we can do something else.
https://github.com/zephyrproject-rtos/zephyr/blob/master/samples/subsys/shell/fs/CMakeLists.txt#L5-L8

@tejlmand
Copy link
Collaborator

tejlmand commented Apr 8, 2020

actually, the CMake code in the linked file will never work as intended, cause APPLICATION_SOURCE_DIR is a variable being defined in boilerplate, and hence not available in code at that time it is used.

tejlmand added a commit to tejlmand/zephyr that referenced this issue Apr 8, 2020
Fixes: zephyrproject-rtos#24158

The shell/fs sample was using APPLICATION_SOURCE_DIR before it has been
defined.

This has now been fixed to use the proper way of overlaying board
specific settings using boards folder.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
carlescufi pushed a commit that referenced this issue Apr 9, 2020
Fixes: #24158

The shell/fs sample was using APPLICATION_SOURCE_DIR before it has been
defined.

This has now been fixed to use the proper way of overlaying board
specific settings using boards folder.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
avisconti pushed a commit to avisconti/zephyr that referenced this issue Apr 15, 2020
Fixes: zephyrproject-rtos#24158

The shell/fs sample was using APPLICATION_SOURCE_DIR before it has been
defined.

This has now been fixed to use the proper way of overlaying board
specific settings using boards folder.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
hakehuang pushed a commit to hakehuang/zephyr that referenced this issue Jun 20, 2020
Fixes: zephyrproject-rtos#24158

The shell/fs sample was using APPLICATION_SOURCE_DIR before it has been
defined.

This has now been fixed to use the proper way of overlaying board
specific settings using boards folder.

Signed-off-by: Torsten Rasmussen <Torsten.Rasmussen@nordicsemi.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Boards bug The issue is a bug, or the PR is fixing a bug platform: nRF Nordic nRFx priority: low Low impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants