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

Kconfig: Expose gnrc/ipv6/whitelist configurations #12893

Conversation

leandrolanzieri
Copy link
Contributor

Contribution description

This moves the gnrc_ipv6_whitelist module configuration macros to the CONFIG_ namespace and exposes them to Kconfig.

Testing procedure

  • Compile gnrc_networking example application using this module (USEMODULE=gnrc_ipv6_whitelist make all term), the default configuration should still be the same.

  • Call menuconfig using this module (USEMODULE=gnrc_ipv6_whitelist make menuconfig), you should the list size as an option. The default list size should still be the same. If you change the value it should change in the app.

Issues/PRs references

Part of #12888

@leandrolanzieri leandrolanzieri 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 Dec 6, 2019
@leandrolanzieri leandrolanzieri added this to the Release 2020.01 milestone Dec 6, 2019
#ifndef GNRC_IPV6_WHITELIST_SIZE
#define GNRC_IPV6_WHITELIST_SIZE (8)
#ifndef CONFIG_GNRC_IPV6_WHITELIST_SIZE
#define CONFIG_GNRC_IPV6_WHITELIST_SIZE (8)
Copy link
Member

Choose a reason for hiding this comment

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

Do we have to rename all config macros to be usable with kconfig?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, Kconfig generates the .config and .h files appending the CONFIG_ symbol. Regardless of that, I think it's good to have configuration macros under a single namespace.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, were just wondering for future integrations.

@jia200x
Copy link
Member

jia200x commented Dec 10, 2019

I set the whitelist size to 100 via menuconfig and applied the following patch:

diff --git a/examples/gnrc_networking/main.c b/examples/gnrc_networking/main.c
index 6301f4291..c9b4b9efe 100644
--- a/examples/gnrc_networking/main.c
+++ b/examples/gnrc_networking/main.c
@@ -39,6 +39,7 @@ int main(void)
      * receive potentially fast incoming networking packets */
     msg_init_queue(_main_msg_queue, MAIN_QUEUE_SIZE);
     puts("RIOT network stack example application");
+    printf("%i\n", CONFIG_GNRC_IPV6_WHITELIST_SIZE);
 
     /* start shell */
     puts("All up, running the shell now");

The output is

main(): This is RIOT! (Version: 2020.01-devel-1311-g9a0a51-copr/12893)
RIOT network stack example application
100
All up, running the shell now

So, it works as expected.

@jia200x
Copy link
Member

jia200x commented Dec 10, 2019

implementation of KConfig files looks also good. It should be easy to proceed.

Copy link
Member

@jia200x jia200x left a comment

Choose a reason for hiding this comment

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

ACK on my side. @miri64 do you have something to add?

@jia200x jia200x added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Dec 10, 2019
@miri64
Copy link
Member

miri64 commented Dec 10, 2019

@miri64 do you have something to add?

Nope all good from my side. Murdock seems to have failed on an unrelated error though.

@miri64 miri64 added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Dec 10, 2019
@miri64 miri64 merged commit d926810 into RIOT-OS:master Dec 10, 2019
@leandrolanzieri leandrolanzieri deleted the pr/kconfig_migrate/gnrc/ipv6_whitelisting branch December 10, 2019 17:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 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
Development

Successfully merging this pull request may close these issues.

3 participants