Skip to content

ieee802154: Expose configurations to Kconfig#13315

Merged
leandrolanzieri merged 3 commits intoRIOT-OS:masterfrom
jia200x:pr/kconfig/ieee802154
Apr 8, 2020
Merged

ieee802154: Expose configurations to Kconfig#13315
leandrolanzieri merged 3 commits intoRIOT-OS:masterfrom
jia200x:pr/kconfig/ieee802154

Conversation

@jia200x
Copy link
Copy Markdown
Member

@jia200x jia200x commented Feb 7, 2020 •
edited
Loading

Contribution description

This PR exposes IEEE802.15.4 configurations to Kconfig

Testing procedure

  • Testing with e.g default and a IEEE802.15.4 radio (e.g at86rf233 on samr21-xpro).
  • Change some settings (Channel, Power, etc) and check that they reflect on the binary (use ifconfig to check these settings)
  • Check that it's still possible to configure the radios without Kconfig (e.g DEFAULT_CHANNEL or CFLAGS)

Issues/PRs references

Part of #12888

@jia200x jia200x requested review from a user, benpicco and leandrolanzieri February 7, 2020 11:07
@jia200x jia200x added the Area: Kconfig Area: Kconfig integration label Feb 7, 2020
@leandrolanzieri leandrolanzieri mentioned this pull request Feb 7, 2020
Open
71 tasks
jia200x
jia200x commented Feb 7, 2020
View reviewed changes
sys/net/link_layer/Kconfig
@@ -0,0 +1,7 @@
# Copyright (c) 2019 HAW Hamburg
Copy link
Copy Markdown
Member Author

@jia200x jia200x Feb 7, 2020

Choose a reason for hiding this comment

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

Oops, I forgot to update the year

leandrolanzieri
leandrolanzieri reviewed Feb 13, 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.

Changes look good. I tested this on the samr21-xpro and configurations are applied correctly. Just to minor comments (plus the year fix in the copyright :)

@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: 3-testing The PR was tested according to the maintainer guidelines labels Feb 13, 2020
@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Feb 13, 2020
@leandrolanzieri leandrolanzieri added the Type: new feature The issue requests / The PR implemements a new feature for RIOT label Feb 13, 2020
benpicco
benpicco reviewed Feb 19, 2020
View reviewed changes
benpicco
benpicco reviewed Feb 19, 2020
View reviewed changes
@tcschmidt
Copy link
Copy Markdown
Member

tcschmidt commented Apr 1, 2020

@jia200x @leandrolanzieri can you fix this?

@jia200x
Copy link
Copy Markdown
Member Author

jia200x commented Apr 8, 2020

I pushed a fixup with the requested changes.

@leandrolanzieri leandrolanzieri added 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 Apr 8, 2020
@leandrolanzieri
Copy link
Copy Markdown
Contributor

leandrolanzieri commented Apr 8, 2020 •
edited
Loading

Changes look good. Please squash.

When you rebase please make a grep to check that all macro names are updated, it seems that you will have to apply changes to the recently added at86rf215.h and kw41zrf.h (at least).

@jia200x jia200x force-pushed the pr/kconfig/ieee802154 branch from 9331c67 to 21ffd44 Compare April 8, 2020 16:28
@jia200x
Copy link
Copy Markdown
Member Author

jia200x commented Apr 8, 2020

done! updated the at86rf215 and kw41zrf

@jia200x jia200x force-pushed the pr/kconfig/ieee802154 branch from 21ffd44 to fe8f83f Compare April 8, 2020 16:51
@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 Apr 8, 2020
leandrolanzieri
leandrolanzieri approved these changes Apr 8, 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.

Changes look good. ACK!

@jia200x
makefiles/default-radio-settings: disable if Kconfig
c712b72 
If Kconfig is used for configuring default parameters of radios, this
logic has to be disabled in order to avoid collisions of config macros
@jia200x jia200x force-pushed the pr/kconfig/ieee802154 branch from fe8f83f to 77325b4 Compare April 8, 2020 17:08
@leandrolanzieri leandrolanzieri merged commit a06d9bb into RIOT-OS:master Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benpicco benpicco benpicco left review comments

@leandrolanzieri leandrolanzieri leandrolanzieri approved these changes

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

@cgundogan cgundogan Awaiting requested review from cgundogan

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

@smlng smlng Awaiting requested review from smlng

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.04

Development

Successfully merging this pull request may close these issues.

4 participants

@jia200x @tcschmidt @leandrolanzieri @benpicco