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

Allow global overlay config files #14740

Closed
jukkar opened this issue Mar 20, 2019 · 11 comments
Closed

Allow global overlay config files #14740

jukkar opened this issue Mar 20, 2019 · 11 comments
Assignees
Labels

Comments

@jukkar
Copy link
Member

jukkar commented Mar 20, 2019

We should have a way to have "global" config overlays. For example when using networking in qemu_x86 it would be nice to use overlay-e1000.conf when compiling zephyr. This overlay is found only in couple of networking samples which is quite annoying actually. The same overlay-e1000.conf could be used in all the samples as it only contains generic options.

I am proposing following:

If user tries to configure the sample like this

cmake .. -DBOARD=qemu_x86 -DOVERLAY_CONFIG=overlay-e1000.conf

but if the overlay-e1000.conf would not be found in local sample directory, a global directory would be searched and the overlay would be used if found there.

@jukkar jukkar added the Enhancement Changes/Updates/Additions to existing features label Mar 20, 2019
@finikorg finikorg pinned this issue Mar 26, 2019
@finikorg finikorg unpinned this issue Mar 26, 2019
@mbolivar
Copy link
Contributor

Perhaps -DCOMMON_OVERLAY_CONFIG=e1000.conf or so? The separate variable could be used to specify "search the directory COMMON_OVERLAY_CONFIG_DIR instead of the current application directory", and this directory would default to something like zephyr/kconfig/common/ (a new directory).

@pfalcon
Copy link
Contributor

pfalcon commented Mar 26, 2019

IMHO, proliferation of config vars isn't a good thing. What would really help is to have "overlay config path", like ".:${ZEPHYR_BASE}/.../overlays".

@jukkar
Copy link
Member Author

jukkar commented Mar 26, 2019

I agree with @pfalcon, it is just very confusing for the user if there are multiple config variables to select the overlay config file.

@mbolivar
Copy link
Contributor

I agree with @pfalcon, it is just very confusing for the user if there are multiple config variables to select the overlay config file.

This introduces the possibility of namespace conflicts, of course. Perhaps a common- prefix could be applied to the ones in the common directory?

Note also having a separate COMMON_OVERLAY_CONFIG_DIR which defaults to somewhere in tree will be necessary to allow out-of-tree common configuration overlays, so I still think it is a good idea. And actually, might as well make it a list, COMMON_OVERLAY_CONFIG_DIRS, to allow site-specific values in addition to in-tree ones.

@overheat
Copy link
Contributor

I have an opposite need for evaluating sensor samples, which is "local" DTS overlays. For example, I need to add overlay files for every board into HTS221 sensor sample, like this:

&arduino_i2c{
hts221@5f {
        compatible = "st,hts221";
        reg = <0x5f>;
        label = "HTS221";
    };
};

But they are the same, if boards support "arduino_i2c" alias. For more detail please refer to "samples\sensor\sht3xd", which sample already included six overlay files in one folder.

Alternative, I can use "-DDTC_OVERLAY_FILE" parameter, but it ask for full path, like this:

west build -b nucleo_f401re samples/sensor/hts221/ -DDTC_OVERLAY_FILE=C:/Users/aaron/zephyrproject/zephyr/samples/sensor/hts221/arduino_i2c.overlay

It's also not convenient for user.

A little wired proposing two things in one thread, but I think they are related. I can open another thread if you consider it's necessary.
So I am proposing a local overlay file:

If the file FOLDER_NAME.overlay exists in your application directory, where FOLDER_NAME is the same as application's directory name, it will be used.

Please take into account this need. Thanks.

@mike-scott
Copy link
Contributor

mike-scott commented Mar 26, 2019

@overheat I believe you are referring to a "shield" setup such as: https://github.com/zephyrproject-rtos/zephyr/tree/master/boards/shields/x_nucleo_iks01a2

This is enabled via
-DSHIELD=x_nucleo_iks01a2

You can setup your shield similarly.

@overheat
Copy link
Contributor

overheat commented Mar 26, 2019

@mike-scott Yes, looks it's more convenient. Thanks!
BTW, what's the mechanism inside, that I didn't specify overlay file with "-DSHIELD=x_nucleo_iks01a2", but it is used? why it only happen under shields folder?
PS:

west build -b nucleo_f401re samples/shields/x_nucleo_iks01a1/

works fine.

@jukkar
Copy link
Member Author

jukkar commented Mar 27, 2019

Perhaps a common- prefix could be applied to the ones in the common directory?

Sure, why not. We can of course name the files as we like here, perhaps we could even name the files without the "overlay" string as that seems to be a bit redundant, so something like common-e1000.conf instead of common-overlay-e1000.conf or overlay-e1000.conf

@pfalcon
Copy link
Contributor

pfalcon commented Mar 27, 2019

Perhaps a common- prefix could be applied to the ones in the common directory?

IMHO, this superfluous and precludes some interesting usecases. Instead, I'd like to draw attention again to "overlay config path" idea, like .:${ZEPHYR_BASE}/.../overlays. Note that current directory is the first entry. That means that some overlay file like overlay-foo-feature.conf would normally be stored in a common overlay dir. But if current app would need to make some additional tweaks to such a common config, the tweaked config could be just to put in the app's dir, all transparent to a user (but definitely with enough build log output to be able to trace what final config file paths are merged into final config).

jukkar added a commit to jukkar/zephyr that referenced this issue Apr 25, 2019
If OVERLAY_CONFIG_DIRS environment variable is specified, then
search it if the file in OVERLAY_CONFIG is not found.

Example:

$ cd samples/net/ipv4_autoconf
$ OVERLAY_CONFIG_DIRS=../sockets/echo_server \
  cmake -B build -DBOARD=qemu_x86 -DOVERLAY_CONFIG=overlay-e1000.conf .

Zephyr version: 1.14.99
-- Selected BOARD qemu_x86
-- Found west: west (found version "0.5.6", minimum required is "0.5.6")
-- Loading zephyr/boards/x86/qemu_x86/qemu_x86.dts as base
-- Overlaying zephyr/dts/common/common.dts
-- Overlaying ../sockets/echo_server/overlay-e1000.conf
Parsing Kconfig tree in zephyr/Kconfig

So in this example the samples/net/ipv4_autoconf directory does
not contain overlay-e1000.conf, but it is found in
../sockets/echo_server/ directory which is mentioned in
OVERLAY_CONFIG_DIRS variable.

Fixes zephyrproject-rtos#14740

Signed-off-by: Jukka Rissanen <[email protected]>
@tejlmand
Copy link
Collaborator

We should have a way to have "global" config overlays. For example when using networking in qemu_x86 it would be nice to use overlay-e1000.conf when compiling zephyr. This overlay is found only in couple of networking samples which is quite annoying actually. The same overlay-e1000.conf could be used in all the samples as it only contains generic options.

Although I do see the value in the proposal, I would like to point out that we should be very careful on such approach.
What config overlays should be allowed in such common folder / what would be the threshold ?

And why are we doing this ?

  • For the end-users
  • For Zephyr maintainers

To end-users, the current solution is clean in the sense that a user can look at:
https://github.com/zephyrproject-rtos/zephyr/blob/main/samples/net/sockets/echo_server/overlay-e1000.conf

and know this kconfig fragment works with this sample.

But if moving such fragment to a common folder, then such relationship is lost, and instead we would need to carefully document when a fragment can be applied.

The Zephyr build system is already quite complex with Kconfig and devicetree files being picked up from a number of different places. There is a learning curve for new developers starting out with Zephyr.
Extending this complexity with more locations will make it even more difficult, so we should be very careful.

That said, the e1000.conf seems very generic, but question is, whether a better Kconfig default scheme would be better.
So that a user building a networking sample can simply do:
cmake -DCONFIG_ETH_E1000=y and other relevant settings defaults to better / more sane values, instead of: cmake -DOVERLAY_CONFIG=common-overlay-e1000.conf`.

I would personally like to see if some of this could improves by better default scheme.

@nashif
Copy link
Member

nashif commented Feb 12, 2024

This is already possible using snippets.

@nashif nashif closed this as completed Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants