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

gnrc/pktdump : Expose Configurations to Kconfig #14071

Merged
merged 2 commits into from
Jun 9, 2020
Merged

gnrc/pktdump : Expose Configurations to Kconfig #14071

merged 2 commits into from
Jun 9, 2020

Conversation

akshaim
Copy link
Member

@akshaim akshaim commented May 13, 2020 •
edited
Loading

Contribution description

This PR exposes compile configurations in PKTDUMP GNRC to Kconfig.

Testing procedure

  1. New documentation was built using doxygen

The build works fine.

  1. Test folder and files were introduced to show the macro on Native environment.

Test Files - Updated

Default State:

Firmware Output

RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.07-devel-608-g1171d8-Kconfig_pktdump_tests)
CONFIG_GNRC_PKTDUMP_MSG_QUEUE_SIZE_EXP=3
GNRC_PKTDUMP_MSG_QUEUE_SIZE=(1<<3)

Usage with CFLAGS

/tests/gnrc_pkdump/Makefile

CFLAGS += -DCONFIG_GNRC_PKTDUMP_MSG_QUEUE_SIZE_EXP=4

Firmware Output

RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.07-devel-608-g1171d8-Kconfig_pktdump_tests)
CONFIG_GNRC_PKTDUMP_MSG_QUEUE_SIZE_EXP=4
GNRC_PKTDUMP_MSG_QUEUE_SIZE=(1<<4)

Usage with Kconfig

/tests/gnrc_pktdump

make menuconfig

Firmware Output

RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

main(): This is RIOT! (Version: 2020.07-devel-608-g1171d8-Kconfig_pktdump_tests)
CONFIG_GNRC_PKTDUMP_MSG_QUEUE_SIZE_EXP=7
GNRC_PKTDUMP_MSG_QUEUE_SIZE=(1<<7)

Issues/PRs references

#12888

@miri64
Copy link
Member

miri64 commented May 14, 2020

Mh.. as far as I can see, except for gnrc_netif the queue sizes in GNRC were not touched so far by Kconfig. @leandrolanzieri is there a reason for that?

@leandrolanzieri
Copy link
Contributor

Mh.. as far as I can see, except for gnrc_netif the queue sizes in GNRC were not touched so far by Kconfig. @leandrolanzieri is there a reason for that?

Which queue sizes are missing from the exposed modules? I see these configurable queue sizes:

  • GNRC_SIXLOWPAN_MSG_QUEUE_SIZE
  • GNRC_IPV6_MSG_QUEUE_SIZE
  • GNRC_NETIF_MSG_QUEUE_SIZE

@miri64
Copy link
Member

miri64 commented May 14, 2020

Which queue sizes are missing from the exposed modules? I see these configurable queue sizes:

* `GNRC_SIXLOWPAN_MSG_QUEUE_SIZE`

* `GNRC_IPV6_MSG_QUEUE_SIZE`

* `GNRC_NETIF_MSG_QUEUE_SIZE`

TCP, UDP and RPL should also be configurable. I must have looked at an older branch, because now I can confirm, that IPv6 and 6LoWPAN are configurable via Kconfig.

@leandrolanzieri
Copy link
Contributor

TCP, UDP and RPL should also be configurable. I must have looked at an older branch, because now I can confirm, that IPv6 and 6LoWPAN are configurable via Kconfig.

RPL is waiting for review #13941, the other modules are yet pending to be exposed.

miri64
miri64 requested changes May 14, 2020
View reviewed changes
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

There should also be a doxygen comment on CONFIG_GNRC_PKTDUMP_MSG_QUEUE_SIZE_EXP

/**
 * @brief Exponent for the queue size (resulting in the queue size 2^n)
 */
#ifdef DOXYGEN
#define CONFIG_GNRC_PKTDUMP_MSG_QUEUE_SIZE_EXP   (4U)
#endif

sys/include/net/gnrc/pktdump.h Outdated Show resolved Hide resolved
sys/net/gnrc/pktdump/Kconfig Outdated Show resolved Hide resolved
sys/include/net/gnrc/pktdump.h Outdated Show resolved Hide resolved
sys/net/gnrc/pktdump/Kconfig Outdated Show resolved Hide resolved
sys/net/gnrc/pktdump/Kconfig Outdated Show resolved Hide resolved
sys/include/net/gnrc/pktdump.h Outdated Show resolved Hide resolved
@miri64 miri64 mentioned this pull request May 14, 2020
Merged
@miri64
Copy link
Member

miri64 commented May 15, 2020

Needs rebase

@akshaim akshaim force-pushed the Kconfig_pktdump branch from cdca1a1 to 7530910 Compare May 15, 2020 16:36
@akshaim
Copy link
Member Author

akshaim commented May 15, 2020

Needs rebase

Done. Updated and tested.

@leandrolanzieri leandrolanzieri added Area: Kconfig Area: Kconfig integration Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT labels May 18, 2020
@leandrolanzieri leandrolanzieri added this to the Release 2020.07 milestone May 18, 2020
@leandrolanzieri leandrolanzieri mentioned this pull request May 20, 2020
Open
71 tasks
@leandrolanzieri
Copy link
Contributor

@miri64 are you OK with the changes?

miri64
miri64 approved these changes Jun 8, 2020
View reviewed changes
Copy link
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Yepp, let's merge this.

@miri64
Copy link
Member

miri64 commented Jun 8, 2020

@akshaim please squash!

@miri64 miri64 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 8, 2020
@akshaim akshaim force-pushed the Kconfig_pktdump branch 2 times, most recently from da4703f to 6d40b44 Compare June 8, 2020 14:11
@akshaim
Copy link
Member Author

akshaim commented Jun 8, 2020

@miri64 I have retained the #ifndef guards as discussed in PR #14086

@miri64
Copy link
Member

miri64 commented Jun 9, 2020

You may squash the style fix immediately

akshaim and others added 2 commits June 9, 2020 13:31
@akshaim
gnrc/pktdump : Update documentation
6b0f822 
Add compile configuration 'GNRC_PKTDUMP_MSG_QUEUE_SIZE' to
'net_gnrc_conf' group
@akshaim @leandrolanzieri
gnrc/pktdump : Add 'CONFIG_' & expose to Kconfig
27e0d7c 
Introduced 'GNRC_PKTDUMP_MSG_QUEUE_SIZE_EXP' to hold exponent
value and made GNRC_PKTDUMP_MSG_QUEUE_SIZE dependant on
GNRC_PKTDUMP_MSG_QUEUE_SIZE_EXP.

Moved 'GNRC_PKTDUMP_MSG_QUEUE_SIZE_EXP' to 'CONFIG_' namespace.

Exposed configurations to Kconfig

Co-authored-by: Martine Lenders <[email protected]>

Co-authored-by: Leandro Lanzieri <[email protected]>
@akshaim akshaim force-pushed the Kconfig_pktdump branch from 6d40b44 to 27e0d7c Compare June 9, 2020 08:02
@akshaim
Copy link
Member Author

akshaim commented Jun 9, 2020

You may squash the style fix immediately

Done.

@miri64 miri64 merged commit 9270f31 into RIOT-OS:master Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miri64 miri64 miri64 approved these changes

@leandrolanzieri leandrolanzieri leandrolanzieri left review comments

@cgundogan cgundogan Awaiting requested review from cgundogan

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

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

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

Assignees
No one assigned
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 Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Milestone
Release 2020.07
Development

Successfully merging this pull request may close these issues.
3 participants
@akshaim @miri64 @leandrolanzieri