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.

Sign up for GitHub

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

Jump to bottom

Kconfig: Expose gnrc/netif configurations #13065

Merged
merged 5 commits into from
Jan 13, 2020
Merged

Kconfig: Expose gnrc/netif configurations #13065

merged 5 commits into from
Jan 13, 2020

Conversation

PeterKietzmann
Copy link
Member

Contribution description

This PR moves the configuration macros of gnrc_netif found here to the CONFIG_ namespace, and exposes them to Kconfig.

As agreed offline with @leandrolanzieri I did not include the default thread priority parameter because this should be defined by the platform, if not changed manually. We might need a better solution to this in future.

Furthermore, I did not include GNRC_NETIF_NUMOF because this won't be static anymore in the future.

Testing procedure

  • examples/gnrc_networking should still work as usual.
  • You should be able to configure the module's parameters via make menuconfig in the test application.

Issues/PRs references

Part of #12888

@PeterKietzmann PeterKietzmann added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT TF: Config Marks issues and PRs related to the work of the Configuration Task Force labels Jan 9, 2020
@leandrolanzieri leandrolanzieri mentioned this pull request Jan 9, 2020
Open
71 tasks
leandrolanzieri
leandrolanzieri requested changes Jan 10, 2020
View reviewed changes
Copy link
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.

It looks good. examples/gnrc_networking builds as usual and configurations can be changed via Kconfig.

Just one comment: you skipped two occurrences of GNRC_NETIF_IPV6_ADDRS_NUMOF:

  • tests/gnrc_sock_dns/Makefile:34
  • examples/gnrc_minimal/Makefile:27

sys/net/gnrc/netif/Kconfig Outdated Show resolved Hide resolved
@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: 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 Jan 10, 2020
@PeterKietzmann
Copy link
Member Author

@leandrolanzieri thanks for the review. I addressed your comments.

@leandrolanzieri leandrolanzieri added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 10, 2020
@leandrolanzieri
Copy link
Contributor

The message for b966ec2 seems to be too long 😕

@PeterKietzmann PeterKietzmann force-pushed the pr_kconf_netif branch 2 times, most recently from 4debfca to cce97a0 Compare January 10, 2020 14:19
@leandrolanzieri leandrolanzieri added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Jan 13, 2020
leandrolanzieri
leandrolanzieri approved these changes Jan 13, 2020
View reviewed changes
Copy link
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.

I did a few greps and all macros appear to be changed now (with the correct prefix). ACK!

@leandrolanzieri leandrolanzieri merged commit 8e6f6ee into RIOT-OS:master Jan 13, 2020
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Jan 15, 2020
This was referenced Jan 21, 2020
Merged
Closed
Merged
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

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

Assignees
No one assigned
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR 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 TF: Config Marks issues and PRs related to the work of the Configuration Task Force Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
Release 2020.01
Development

Successfully merging this pull request may close these issues.
4 participants
@PeterKietzmann @leandrolanzieri @miri64 @fjmolinas