Skip to content

Kconfig: source EXTERNAL_MODULES configurations#16053

Merged
leandrolanzieri merged 2 commits intoRIOT-OS:masterfrom
fjmolinas:pr_kconfig_external_modules
Feb 24, 2021
Merged

Kconfig: source EXTERNAL_MODULES configurations#16053
leandrolanzieri merged 2 commits intoRIOT-OS:masterfrom
fjmolinas:pr_kconfig_external_modules

Conversation

@fjmolinas
Copy link
Copy Markdown
Contributor

@fjmolinas fjmolinas commented Feb 19, 2021

Contribution description

This PR enables handling external module configurations via Kconfig. They are all included under the same external modules menu.

To be able to source them easily as a blob all EXTERNAL_MODULE_DIRS that hold a configuration file are written as osource <dir_path>/Kconfig to a Kconfig.external_modules file.

Testing procedure

  • Test cases added to extend tests/kconfig.

@fjmolinas fjmolinas added Area: build system Area: Build system Area: Kconfig Area: Kconfig integration labels Feb 19, 2021
miri64
miri64 reviewed Feb 19, 2021
View reviewed changes
tests/kconfig/external_module_1/include/external_module_1.h Outdated
@@ -0,0 +1,36 @@
/*
* Copyright (C) 2018 Freie Universität Berlin
Copy link
Copy Markdown
Member

@miri64 miri64 Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this copyright correct?

tests/kconfig/external_module_1/include/external_module_1.h Outdated
* @file
* @brief
*
* @author Gaëtan Harter <gaetan.harter@fu-berlin.de>
Copy link
Copy Markdown
Member

@miri64 miri64 Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this authorship?

@fjmolinas fjmolinas force-pushed the pr_kconfig_external_modules branch from 5ca65dd to 30fcd4c Compare February 19, 2021 14:23
fjmolinas
fjmolinas commented Feb 19, 2021
View reviewed changes
makefiles/kconfig.mk
export KCONFIG_GENERATED_DEPENDENCIES = $(GENERATED_DIR)/Kconfig.dep

# This file will contain external module configurations
export KCONFIG_EXTERNAL_CONFIGS = $(GENERATED_DIR)/Kconfig.external_modules
Copy link
Copy Markdown
Contributor Author

@fjmolinas fjmolinas Feb 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These could be conditionally export, same for the above, but I would rather do both as a cleanup in a followup.

@leandrolanzieri leandrolanzieri added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Feb 19, 2021
leandrolanzieri
leandrolanzieri reviewed Feb 19, 2021
View reviewed changes
@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Feb 22, 2021

I think I addressed your change suggestions @leandrolanzieri!

@leandrolanzieri
Copy link
Copy Markdown
Contributor

leandrolanzieri commented Feb 22, 2021

Looks good @fjmolinas ! Please squash!

@leandrolanzieri leandrolanzieri added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Feb 22, 2021
@fjmolinas fjmolinas force-pushed the pr_kconfig_external_modules branch from 9ce1feb to 22b74aa Compare February 22, 2021 12:33
@fjmolinas fjmolinas added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 22, 2021
@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Feb 22, 2021

@leandrolanzieri murdock succeded, buy you might want to take a look at the latest changes.

@leandrolanzieri
Copy link
Copy Markdown
Contributor

leandrolanzieri commented Feb 23, 2021

@leandrolanzieri murdock succeded, buy you might want to take a look at the latest changes.

Looks good! please squash!

@fjmolinas fjmolinas force-pushed the pr_kconfig_external_modules branch from fd70557 to ab457ab Compare February 23, 2021 18:16
@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Feb 23, 2021

Squashed!

@fjmolinas
Copy link
Copy Markdown
Contributor Author

fjmolinas commented Feb 24, 2021

All green @leandrolanzieri!

leandrolanzieri
leandrolanzieri approved these changes Feb 24, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@leandrolanzieri leandrolanzieri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This provides a clean solution to integrate Kconfig files from external modules. When no modules are used it does not affect the process so this is completely optional. ACK!

@leandrolanzieri leandrolanzieri merged commit d5fc632 into RIOT-OS:master Feb 24, 2021
@kaspar030 kaspar030 added this to the Release 2021.04 milestone Apr 23, 2021
@leandrolanzieri leandrolanzieri added the Release notes: added Set on PRs that have been processed into the release notes for the current release. label Apr 28, 2021
@fjmolinas fjmolinas deleted the pr_kconfig_external_modules branch July 30, 2021 11:28
@NikLeberg NikLeberg mentioned this pull request Jan 31, 2022
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 left review comments

@leandrolanzieri leandrolanzieri leandrolanzieri approved these changes

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@cgundogan cgundogan Awaiting requested review from cgundogan

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@smlng smlng Awaiting requested review from smlng

Assignees

No one assigned

Labels

Area: build system Area: Build system Area: Kconfig Area: Kconfig integration CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Release notes: added Set on PRs that have been processed into the release notes for the current release. Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Milestone

Release 2021.04

Development

Successfully merging this pull request may close these issues.

4 participants

@fjmolinas @leandrolanzieri @miri64 @kaspar030