Skip to content

gnrc/rpl: Expose configurations to Kconfig#13941

Merged
cgundogan merged 22 commits intoRIOT-OS:masterfrom
leandrolanzieri:pr/kconfig_migrate/net/gnrc/rpl
May 14, 2020
Merged

gnrc/rpl: Expose configurations to Kconfig#13941
cgundogan merged 22 commits intoRIOT-OS:masterfrom
leandrolanzieri:pr/kconfig_migrate/net/gnrc/rpl

Conversation

@leandrolanzieri
Copy link
Copy Markdown
Contributor

Contribution description

This PR moves configuration macros of GNRC RPL module to CONFIG_ namespace and exposes them to Kconfig.

Testing procedure

  • Examples using rpl should work as usual
  • Changing configurations via Kconfig (e.g. using menuconfig) should reflect on the configuration macros.

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 Area: Kconfig Area: Kconfig integration labels Apr 24, 2020
@leandrolanzieri leandrolanzieri added this to the Release 2020.07 milestone Apr 24, 2020
@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/net/gnrc/rpl branch from 90ca355 to 06e7932 Compare May 11, 2020 12:45
@cgundogan
Copy link
Copy Markdown
Member

@leandrolanzieri code-wise the changes look correct. I wanted to give this a test, but the RPL menu does not appear in the menu? Is an include missing?

@cgundogan cgundogan 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 May 14, 2020
@leandrolanzieri
Copy link
Copy Markdown
Contributor Author

Is an include missing?

Indeed, please give it a try now :-)

Copy link
Copy Markdown
Member

@cgundogan cgundogan left a comment

Choose a reason for hiding this comment

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

Thanks! Tested and works like a charm!

@cgundogan
Copy link
Copy Markdown
Member

and please squash.

@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/net/gnrc/rpl branch from 61184a9 to 4b3bfd3 Compare May 14, 2020 13:57
@leandrolanzieri
Copy link
Copy Markdown
Contributor Author

Done

@cgundogan cgundogan added Reviewed: 3-testing The PR was tested according to the maintainer guidelines CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels May 14, 2020
#ifndef GNRC_RPL_MSG_QUEUE_SIZE
#define GNRC_RPL_MSG_QUEUE_SIZE (8U)
#ifndef CONFIG_GNRC_RPL_MSG_QUEUE_SIZE
#define CONFIG_GNRC_RPL_MSG_QUEUE_SIZE (8U)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In #14071 the exponent was used to model the power of 2 restriction of message queues. Should we do this here as well or rather as a follow-up (including the other queue sizes)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good point. Let's defer that to a follow-up PR, though. The diff and commit history is already kind of convoluted.

@cgundogan cgundogan merged commit dd61040 into RIOT-OS:master May 14, 2020
@leandrolanzieri leandrolanzieri deleted the pr/kconfig_migrate/net/gnrc/rpl branch May 14, 2020 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: Kconfig Area: Kconfig integration 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 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