Skip to content

gnrc/mac : Expose configurations to Kconfig#14138

Merged
leandrolanzieri merged 8 commits intoRIOT-OS:masterfrom
akshaim:Kconfig_mac
Jun 17, 2020
Merged

gnrc/mac : Expose configurations to Kconfig#14138
leandrolanzieri merged 8 commits intoRIOT-OS:masterfrom
akshaim:Kconfig_mac

Conversation

@akshaim
Copy link
Copy Markdown
Member

@akshaim akshaim commented May 25, 2020

Contribution description

This PR exposes compile configurations in GNRC: MAC to Kconfig.

Testing procedure

  1. New documentation was built using Doxygen

The build works fine.

  1. Macro was introduced
#define STR(x)   #x
#define SHOW_DEFINE(x) printf("%s=%s\n", #x, STR(x))

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-739-g6e24-Kconfig_mac_tests)
CONFIG_GNRC_MAC_RX_QUEUE_SIZE_EXP=(3U)
GNRC_MAC_RX_QUEUE_SIZE=(1 << (3U))
CONFIG_GNRC_MAC_DISPATCH_BUFFER_SIZE_EXP=(3U)
GNRC_MAC_DISPATCH_BUFFER_SIZE=(1 << (3U))
CONFIG_GNRC_MAC_NEIGHBOR_COUNT=4
CONFIG_GNRC_MAC_TX_QUEUE_SIZE_EXP=(3U)
GNRC_MAC_TX_QUEUE_SIZE=4
CONFIG_GNRC_MAC_DISABLE_DUTYCYCLE_RECORD=CONFIG_GNRC_MAC_DISABLE_DUTYCYCLE_RECORD
GNRC_MAC_ENABLE_DUTYCYCLE_RECORD=(1U)

Usage with menuconfig [default values]

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-739-g6e24-Kconfig_mac_tests)
CONFIG_GNRC_MAC_RX_QUEUE_SIZE_EXP=3
GNRC_MAC_RX_QUEUE_SIZE=(1 << 3)
CONFIG_GNRC_MAC_DISPATCH_BUFFER_SIZE_EXP=3
GNRC_MAC_DISPATCH_BUFFER_SIZE=(1 << 3)
CONFIG_GNRC_MAC_NEIGHBOR_COUNT=8
CONFIG_GNRC_MAC_TX_QUEUE_SIZE_EXP=3
GNRC_MAC_TX_QUEUE_SIZE=(1 << 3)
CONFIG_GNRC_MAC_DISABLE_DUTYCYCLE_RECORD=CONFIG_GNRC_MAC_DISABLE_DUTYCYCLE_RECORD
GNRC_MAC_ENABLE_DUTYCYCLE_RECORD=(1U)

Usage with menuconfig

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-739-g6e24-Kconfig_mac_tests)
CONFIG_GNRC_MAC_RX_QUEUE_SIZE_EXP=4
GNRC_MAC_RX_QUEUE_SIZE=(1 << 4)
CONFIG_GNRC_MAC_DISPATCH_BUFFER_SIZE_EXP=2
GNRC_MAC_DISPATCH_BUFFER_SIZE=(1 << 2)
CONFIG_GNRC_MAC_NEIGHBOR_COUNT=4
CONFIG_GNRC_MAC_TX_QUEUE_SIZE_EXP=2
GNRC_MAC_TX_QUEUE_SIZE=(1 << 2)
CONFIG_GNRC_MAC_DISABLE_DUTYCYCLE_RECORD=1
GNRC_MAC_ENABLE_DUTYCYCLE_RECORD=(0)

MACROS were successfully configured.

Issues/PRs references

#12888

@akshaim
Copy link
Copy Markdown
Member Author

akshaim commented May 25, 2020

@miri64 Do you think that GNRC_MAC_TYPE_GET_DUTYCYCLE should be made configurable as well ?

@miri64
Copy link
Copy Markdown
Member

miri64 commented May 25, 2020

@miri64 Do you think that GNRC_MAC_TYPE_GET_DUTYCYCLE should be made configurable as well ?

No, its an IPC message type. It is unclear to me, why it is configurable in the current version in the first place.

@akshaim
Copy link
Copy Markdown
Member Author

akshaim commented May 25, 2020

@miri64 Do you think that GNRC_MAC_TYPE_GET_DUTYCYCLE should be made configurable as well ?

No, its an IPC message type. It is unclear to me, why it is configurable in the current version in the first place.

If you think so, maybe I should remove the ifndef statements associated with that macro.

@miri64
Copy link
Copy Markdown
Member

miri64 commented May 25, 2020

@miri64 Do you think that GNRC_MAC_TYPE_GET_DUTYCYCLE should be made configurable as well ?

No, its an IPC message type. It is unclear to me, why it is configurable in the current version in the first place.

If you think so, maybe I should remove the ifndef statements associated with that macro.

Yes, but this is IMHO a separate change from the Kconfig integration.

@akshaim akshaim closed this May 26, 2020
@akshaim akshaim reopened this May 26, 2020
@akshaim
Copy link
Copy Markdown
Member Author

akshaim commented May 26, 2020

@miri64 Okay. I shall go for a separate PR then.

@leandrolanzieri leandrolanzieri mentioned this pull request May 28, 2020
Open
71 tasks
@leandrolanzieri leandrolanzieri added Area: Kconfig Area: Kconfig integration Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Jun 11, 2020
leandrolanzieri
leandrolanzieri requested changes Jun 11, 2020
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.

Some minor comments. Besides that I'd rather have just one commit that updates the documentation after all changes are done. That would be clearer.

@akshaim
Copy link
Copy Markdown
Member Author

akshaim commented Jun 11, 2020 •
edited
Loading

Some minor comments. Besides that I'd rather have just one commit that updates the documentation after all changes are done. That would be clearer.

Sure. I shall do that from now on. I used ';' to restrict the line to 80 chars.

@leandrolanzieri
Copy link
Copy Markdown
Contributor

leandrolanzieri commented Jun 11, 2020

@akshaim please squash, and try merging the documentation commits

@akshaim
Copy link
Copy Markdown
Member Author

akshaim commented Jun 11, 2020

@leandrolanzieri Please wait. Need to test.

@akshaim
Copy link
Copy Markdown
Member Author

akshaim commented Jun 12, 2020

@leandrolanzieri Please wait. Need to test.

Done.

@leandrolanzieri
Copy link
Copy Markdown
Contributor

leandrolanzieri commented Jun 15, 2020

I think something happened when squashing. Why is GNRC_MAC_DISPATCH_BUFFER_SIZE being removed in 0921e6d?

akshaim and others added 3 commits June 15, 2020 12:28
@akshaim @leandrolanzieri
gnrc/mac : Expose to Kconfig
25f778b 
Expose configurations to Kconfig

Co-authored-by: Leandro Lanzieri <leandro.lanzieri@haw-hamburg.de>
@akshaim
tests/unittests/mac : Added check for macro if set via Kconfig
414a0b0
@akshaim
gnrc/mac : Update documentation
383555c 
Move non 'CONFIG_' macros away from 'net_gnrc_mac_conf' group
@akshaim
Copy link
Copy Markdown
Member Author

akshaim commented Jun 15, 2020

I think something happened when squashing. Why is GNRC_MAC_DISPATCH_BUFFER_SIZE being removed in 0921e6d?

Thanks for pointing that out. Multiple rebases to merge the documentation resulted in some unexpected changes. The final commit passed the tests however I missed the intermediate ones.

@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 CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR 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 Jun 16, 2020
leandrolanzieri
leandrolanzieri approved these changes Jun 17, 2020
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.

ACK.

@leandrolanzieri leandrolanzieri merged commit 3ce8efd into RIOT-OS:master Jun 17, 2020
@miri64 miri64 added this to the Release 2020.07 milestone Jun 24, 2020
@akshaim akshaim mentioned this pull request Jul 21, 2020
Merged
@akshaim akshaim deleted the Kconfig_mac branch July 23, 2020 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@leandrolanzieri leandrolanzieri leandrolanzieri approved these changes

@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

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

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

Assignees

No one assigned

Labels

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 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 2020.07

Development

Successfully merging this pull request may close these issues.

3 participants

@akshaim @miri64 @leandrolanzieri