Board-specific overlays defined in a module's shield directory are not found

[cdwilson]

Describe the bug
This issue is similar to #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/:

boards/shields/adafruit_neopixel_grid_bff/boards/
├── adafruit_qt_py_rp2040.conf
└── adafruit_qt_py_rp2040.overlay

However, if a boards/shields/ directory is defined in a Zephyr module, the board-specific overlays within the shield's boards/ directory are not found:

<zephyr-workspace>/modules/lib/golioth-zephyr-boards/
├── boards
│   ├── ...
│   └── shields
│       └── arduino_uno_click
│           └── boards
│               ├── nrf9160dk_nrf9160.overlay <-- not found
│               ├── nrf9160dk_nrf9160_common.dtsi <-- not found
│               └── nrf9160dk_nrf9160_ns.overlay <-- not found
└── zephyr
    └── module.yml

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 an arduino_uno_click shield installed on the nRF9160dk board.

After checking out each branch below, the firmware can be built using the following command:

west build -p -b nrf9160dk/nrf9160/ns samples/sensor/bme280 -- -DSHIELD="arduino_uno_click mikroe_weather_click"

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:

...
-- Found devicetree overlay: <redacted>/zephyr/boards/shields/arduino_uno_click/arduino_uno_click.overlay
-- Found devicetree overlay: <redacted>/zephyr/boards/shields/arduino_uno_click/boards/nrf9160dk_nrf9160.overlay
-- Found devicetree overlay: <redacted>/zephyr/boards/shields/arduino_uno_click/boards/nrf9160dk_nrf9160_ns.overlay
-- Found devicetree overlay: <redacted>/zephyr/boards/shields/mikroe_weather_click/mikroe_weather_click.overlay
...

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 the arduino_uno_click shield, but located within the module's boards/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:

...
-- Found devicetree overlay: <redacted>/zephyr/boards/shields/arduino_uno_click/arduino_uno_click.overlay
-- Found devicetree overlay: <redacted>/zephyr/boards/shields/mikroe_weather_click/mikroe_weather_click.overlay
...

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 the shields/ directory in the Zephyr tree when searching for overlay files. In the example above, I expect that overlays in boards/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):

• OS: macOS 14.4.1
• Toolchain Zephyr SDK: zephyr-sdk-0.16.5-1
• Commit SHA or Version used: f643f3a

[MaroMetelski]

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.

[cdwilson]

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 #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.

[aescolar]

@tejlmand or @nordicjm , @57300 can you please take a look at this?

[tejlmand]

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: #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)

[cdwilson]

@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:

1. I want to be able to modify/extend an existing "in-tree" board definition
2. I want to be able to modify/extend an existing "in-tree" shield definition
3. 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).
4. I want to be able to do all three of the above in a repo separate from my app repo (i.e. I want to be able to make the change in one place and pull that change into multiple apps via west.yml)

[digiexchris]

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.

[tejlmand]

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: #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

[tejlmand]

Here is what I'd like to be able to achieve at a high-level:

1. 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.

2. 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: #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.

3. 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.

[digiexchris]

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: #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

[tejlmand]

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.

[digiexchris]

thanks! this solves a lot of problems (unrelated to this too)!

[carlescufi]

@cdwilson could you confirm that #72857 solves the issue described here?

[tejlmand]

@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]

@cdwilson could you confirm that #72857 solves the issue described here?

@carlescufi sorry for the delay in response, I'm still trying to digest #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).

[carlescufi]

@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.

[cdwilson]

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)