Open cdwilson opened 5 months ago
Hello! Let me jump in as I quickly glanced over this issue.
As far as I see your problem is specific to the case when you are trying to add board-specific overlays for a shield that already exists somewhere else.
You are adding shields/arduino_uno_click/boards/*
for a shield that's defined in Zephyr (zephyr/boards/shields/arduino_uno_click).
It seems that the shields.cmake only allows adding overlays in the same place the board is defined (whether in-tree or in a Zephyr module).
I expect that the shields/ directory of a Zephyr module are searched exactly like the shields/ directory in the Zephyr tree when searching for overlay files.
The extension still correctly applies board-specific overlays for shields defined in a module (out-of-tree). The requirement is that's the same place that the shield is defined (contains Kconfig.shield
file). Not sure if that's actually a bug since what you are trying to do is somewhat different.
So the question really is whether the script should be able to find ANY boards/shields/arduino_uno_click
(or any other shield for that matter) and apply all the overlays it finds so that you can edit existing shields.
As far as I see your problem is specific to the case when you are trying to add board-specific overlays for a shield that already exists somewhere else.
@MaroMetelski that's correct. In this specific case, I'm trying to apply a board-specific overlay to the arduino_uno_click
shield that already exists in the Zephyr tree (eventually I'm hoping this overlay can be pulled into Zephyr via https://github.com/zephyrproject-rtos/zephyr/pull/71106). Instead of adding this same overlay code to the <board>.overlay
in multiple apps, I attempted to put this in a module that could be reused (but found that it didn't work).
The requirement is that's the same place that the shield is defined (contains Kconfig.shield file). Not sure if that's actually a bug since what you are trying to do is somewhat different. ... So the question really is whether the script should be able to find ANY boards/shields/arduino_uno_click (or any other shield for that matter) and apply all the overlays it finds so that you can edit existing shields.
Thanks for the clarification on this. If this is working as intended, then this may be more of a feature request than a bug. I think it's valuable to be able to modify the built-in boards/shields via overlay/Kconfigs defined in other places (like modules). I'm not familiar enough with how the build system works to know if there are any downsides of doing this though.
@tejlmand or @nordicjm , @57300 can you please take a look at this?
yes, this is more of a feature request, and a good one.
In many ways it is similar in nature to the wish of extending a board in HWMv2: https://github.com/zephyrproject-rtos/zephyr/issues/69548
Now, with the requested behavior, then one thing we need to carefully consider is the behavior of shields. If we allow board overlays for a given existing shield to be defined out-side the location of the shield definition (that is in the module as proposed in thie case) then we can experience some very surprising behaviors.
For example building cmake -DBOARD=plank -DSHIELD=foo <sample>
produces one result.
Now, if I then suddenly do a west config manifest.project-filter -- +bar_module
, and that module extends with extra overlays for plank
when using shield foo
, then I could end with a completely different result.
This suddenly makes it harder to predict which overlays will be picked up, as well as where to look for such knowledge (should I examine all Zephyr modules myself to avoid surprises).
Secondly, users cannot opt-out of those extra overlays, unless they disable the bar_module
.
But bar_module
could contain other features my project needs.
I will change this into a feature request for now, and create a more general shield enhancement meta issue, as there are a couple of more use-cases we would like to support, and where the transition to HWMv2 makes it possible. (Shields has not changes in HWMv2, but shields could benefit from some of the work introduced there)
@tejlmand thanks for the insight on this.
then I could end with a completely different result.
FWIW, this is actually the result I'm looking to achieve with this feature request in this specific case, but I can see that there are cases where this would not be desired.
This suddenly makes it harder to predict which overlays will be picked up, as well as where to look for such knowledge (should I examine all Zephyr modules myself to avoid surprises).
I realized from your comment that this concern applies more generally to including any overlays in a module for boards/shields that are not defined in that same module (not just the specific use case I described in the original issue). i.e. I had not realized that it was not currently possible to include a <board>.overlay
in a module if the <board>
was defined elsewhere.
Here is what I'd like to be able to achieve at a high-level:
west.yml
)I'd like to add one little addition, I'm trying to use an ssd1306 shield with a board that doesn't have arduino_i2c nodes. I can add it easily enough in the zephyr provided board dts for the blackpill_f401ce but I'd rather not modify those. I have an overlay file in my workspace that I'm doing configuration with, so I thought I could add the arduino_i2c name or alias there. The problem is it seems the shield overlay is being processed before my board overlay, so it still complains about not finding that node. I'm not sure if your proposed change will help with this, but I'd like to:
modify an existing "in-tree" board definition to fill in the gaps required by a shield definition. I want to store this change in my app repo.
modify an existing "in-tree" board definition to fill in the gaps required by a shield definition. I want to store this change in my app repo.
@digiexchris take a look here: https://github.com/zephyrproject-rtos/zephyr/pull/72857
This PR will allow you to extend the blackpill_f401ce
board in your own repo like this:
board:
extend: blackpill_f401ce
variants:
- name: myvariant
qualifier: stm32f401xe
and then in your custom blackpill_f401ce_stm32f401xe_myvariant.dts
file add the extra devicetree nodes you require.
Then when building, instead of doing west build -b blackpill_f401ce/stm32f401xe <sample> -- -DSHIELD=ssd1306_128x64
you can now do:
west build -b blackpill_f401ce/stm32f401xe/myvariant <sample> -- -DSHIELD=ssd1306_128x64
or the short form:
west build -b blackpill_f401ce//myvariant <sample> -- -DSHIELD=ssd1306_128x64
Here is what I'd like to be able to achieve at a high-level:
- I want to be able to modify/extend an existing "in-tree" board definition
and this is covered by #72857. Please take a look and provide feedback.
- I want to be able to modify/extend an existing "in-tree" shield definition
Yes, and with the new hardware model in place, then at some point we should consider if the new principle used by boards should be expanded to cover shields. But we have quite a few identified enhancements already: https://github.com/zephyrproject-rtos/zephyr/issues/69546
But feel free to open an enhancement issue to have HWMv2 also covering shields. It sounds like a good candidate for future work and having the new hardware model cover shields makes a lot of sense.
- I want to be able to modify/extend an existing "in-tree" board definition but ONLY when a specific shield is also applied (this was my original issue).
and this is why a shield can define board overlays, so that the board devicetree is adjusted when using the given shield. But I understand that the board overlay provided with the shield itself, does not cover your use-case.
modify an existing "in-tree" board definition to fill in the gaps required by a shield definition. I want to store this change in my app repo.
@digiexchris take a look here: https://github.com/zephyrproject-rtos/zephyr/pull/72857
This PR will allow you to extend the
blackpill_f401ce
board in your own repo like this:board: extend: blackpill_f401ce variants: - name: myvariant qualifier: stm32f401xe
and then in your custom
blackpill_f401ce_stm32f401xe_myvariant.dts
file add the extra devicetree nodes you require.Then when building, instead of doing
west build -b blackpill_f401ce/stm32f401xe <sample> -- -DSHIELD=ssd1306_128x64
you can now do:west build -b blackpill_f401ce/stm32f401xe/myvariant <sample> -- -DSHIELD=ssd1306_128x64
or the short form:
west build -b blackpill_f401ce//myvariant <sample> -- -DSHIELD=ssd1306_128x64
does that solve the problem with the shield dts being processed before my project overlay resulting in the new nodes not being found? Sounds like it would since it would effectively act the same as a custom board
does that solve the problem with the shield dts being processed before my project overlay resulting in the new nodes not being found? Sounds like it would since it would effectively act the same as a custom board
@digiexchris it would, because your extended board variant will define the nodes before the shield overlay is applied.
thanks! this solves a lot of problems (unrelated to this too)!
@cdwilson could you confirm that https://github.com/zephyrproject-rtos/zephyr/pull/72857 solves the issue described here?
@cdwilson getting back to this issue.
I came to notice that you're actually creating overlay files for the nrf9160dk to allow that devkit to work with the uno click shield. The Zephyr in-tree uno click implementation has no overlay's foir the nRF9160dk: https://github.com/zephyrproject-rtos/zephyr/tree/main/boards/shields/arduino_uno_click
So why are you not contributing your overlay files for the nRF9160dk to the Zephyr repository ? I'm quite sure that having in-tree support for uno click on the nRF9160dk will be valuable to other users.
@cdwilson could you confirm that #72857 solves the issue described here?
@carlescufi sorry for the delay in response, I'm still trying to digest https://github.com/zephyrproject-rtos/zephyr/pull/72857 and understand how I would accomplish this using extensions.
I think what you are suggesting is that by using extensions, I can define a new board variant which extends nrf9160dk/nrf9160
by including the content of my nrf9160dk_nrf9160_common.dtsi, something like this:
board:
extend: nrf9160dk
variants:
- name: arduino_uno_click
qualifier: nrf9160
However, in this specific example, I'm building for nrf9160dk/nrf9160/ns
which (as I understand it) is already using the ns
board variant.
It's not clear to me how I would specify both my arduino_uno_click
extension variant and the built-in ns
variant in the same west build
command. e.g.
west build -p -b nrf9160dk/nrf9160/<WHAT GOES HERE???> samples/sensor/bme280 -- -DSHIELD="arduino_uno_click mikroe_weather_click"
Could you explain a bit further what this might look like using board extensions?
So why are you not contributing your overlay files for the nRF9160dk to the Zephyr repository ?
@tejlmand I opened this PR about 2mo ago with this exact overlay, but it's still waiting for a review. Actually, the reason why I originally opened this issue is because I wanted the ability to distribute this overlay in one of our Zephyr modules as a stopgap. I assumed it will take a couple months to get this PR reviewed & merged into the official Zephyr repo, then wait for those changes to get picked up into a new NCS release, then wait for that NCS release to get picked up into a new golioth-firmware-sdk release before it actually becomes available for me to use in an app... 🫠
Btw, not trying to throw shade on anybody for the delay! I really appreciate all the work from maintainers to review PRs, I just know these things take some time to upstream. In general, I'd like to have a workflow where I can extend "in tree" boards/shields via a Zephyr module where I can make changes available immediately to my app(s), and then upstream those changes in parallel (if they are relevant to the broader Zephyr community).
@tejlmand I opened this PR about 2mo ago with this exact overlay, but it's still waiting for a review. Actually, the reason why I originally opened this issue is because I wanted the ability to distribute this overlay in one of our Zephyr modules as a stopgap. I assumed it will take a couple months to get this PR reviewed & merged into the official Zephyr repo, then wait for those changes to get picked up into a new NCS release, then wait for that NCS release to get picked up into a new golioth-firmware-sdk release before it actually becomes available for me to use in an app... 🫠
Sorry about that, this PR fell through the cracks. @erwango has reviewed it now, so please do address his review comments.
Sorry about that, this PR fell through the cracks. @erwango has reviewed it now, so please do address his review comments.
No worries, thanks to you both for helping to get this merged!
@carlescufi I also wanted to follow up on my previous question about how to extend ns
board variants with the new extensions PR (I assume that in general people will want to use extensions with ns
variants of boards, and was just curious how y'all envisioned this working with extensions)
Describe the bug This issue is similar to https://github.com/zephyrproject-rtos/zephyr/issues/67227 but has to do with board-specific overlays defined within a shield directory in a Zephyr module.
Zephyr allows board-specific overlays & Kconfig to be defined for a particular shield by adding a
boards/
directory within the shield's directory, e.g.boards/shields/adafruit_neopixel_grid_bff/boards/
:However, if a
boards/shields/
directory is defined in a Zephyr module, the board-specific overlays within the shield'sboards/
directory are not found:To Reproduce
I have prepared two example branches in my Zephyr fork that can be used to reproduce this using a
mikroe_weather_click
shield installed on anarduino_uno_click
shield installed on thenRF9160dk
board.After checking out each branch below, the firmware can be built using the following command:
Example where overlays are correctly found
The first branch places the board-specific overlays for the
arduino_uno_click
shield in the Zephyr tree where they are correctly found by the build system:https://github.com/cgnd/zephyr/tree/cdwilson/github-issues/zephyr-in-tree-shield-board-overlays/boards/shields/arduino_uno_click/boards
In the build output, the following shield overlays are found:
Example where overlays are NOT correctly found
The second branch pulls in a 3rd-party Zephyr module in
west.yml
that contains the exact same board-specific overlays for thearduino_uno_click
shield, but located within the module'sboards/shields
directory:https://github.com/cgnd/zephyr/blob/cdwilson/github-issues/zephyr-module-shield-board-overlays/west.yml#L32-L35
In the build output, the following shield overlays are found:
In this 2nd example, the
boards/nrf9160dk_nrf9160*.overlay
files defined in the module are not found.Expected behavior I expect that the
shields/
directory of a Zephyr module are searched exactly like theshields/
directory in the Zephyr tree when searching for overlay files. In the example above, I expect that overlays inboards/shields/<shield>/boards/*.overlay
are found during build.Impact Current workaround is to add the overlay code in the app-specific board overlay instead of in the Zephyr module.
Environment (please complete the following information):