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

net/lora : Expose configurations to Kconfig #15007

Merged
merged 15 commits into from
Oct 2, 2020
Merged

net/lora : Expose configurations to Kconfig #15007

merged 15 commits into from
Oct 2, 2020

Conversation

akshaim
Copy link
Member

@akshaim akshaim commented Sep 11, 2020 •
edited
Loading

Contribution description -

This PR exposes compile configurations in net/lora to Kconfig.

Skipped LORA_FREQUENCY_RESOLUTION_DEFAULT since it was planned to be removed. #13318

Testing procedure

  1. New documentation was built using Doxygen

    The build works fine.

  2. Test files were added to tests/net_lora

    The test file can be found here

    Compiled binaries for b-l072z-lrwan1 and tested on the evaluation board.

  3. Tested gnrc_lorawan example in examples

Default State:

Firmware Output
main(): This is RIOT! (Version: 2020.10-devel-1308-gae087-Kconfig_lora_tests)
CONFIG_LORA_PREAMBLE_LENGTH_DEFAULT=(8U)
CONFIG_LORA_SYMBOL_TIMEOUT_DEFAULT=(10U)
CONFIG_LORA_BW_DEFAULT=(LORA_BW_125_KHZ)
CONFIG_LORA_SF_DEFAULT=(LORA_SF7)
CONFIG_LORA_CR_DEFAULT=(LORA_CR_4_5)
CONFIG_LORA_IQ_INVERTED_DEFAULT=CONFIG_LORA_IQ_INVERTED_DEFAULT
CONFIG_LORA_FREQUENCY_HOPPING_DEFAULT=CONFIG_LORA_FREQUENCY_HOPPING_DEFAULT
CONFIG_LORA_FREQUENCY_HOPPING_PERIOD_DEFAULT=(0U)
CONFIG_LORA_FIXED_HEADER_LEN_MODE_DEFAULT=CONFIG_LORA_FIXED_HEADER_LEN_MODE_DEFAULT
CONFIG_LORA_PAYLOAD_CRC_OFF_DEFAULT=CONFIG_LORA_PAYLOAD_CRC_OFF_DEFAULT
LORA_PAYLOAD_CRC_ON_DEFAULT=(true)
CONFIG_LORA_PAYLOAD_LENGTH_DEFAULT=(0U)

Usage with CFLAGS :

CFLAGS += -DCONFIG_LORA_PREAMBLE_LENGTH_DEFAULT=10
CFLAGS += -DCONFIG_LORA_SYMBOL_TIMEOUT_DEFAULT=20
CFLAGS += -DCONFIG_LORA_BW_DEFAULT=LORA_BW_250_KHZ
CFLAGS += -DCONFIG_LORA_SF_DEFAULT=LORA_SF8
CFLAGS += -DCONFIG_LORA_CR_DEFAULT=LORA_CR_4_6
CFLAGS += -DCONFIG_LORA_IQ_INVERTED_DEFAULT=1
CFLAGS += -DCONFIG_LORA_FREQUENCY_HOPPING_DEFAULT=1
CFLAGS += -DCONFIG_LORA_FREQUENCY_HOPPING_PERIOD_DEFAULT=10
# CFLAGS += -DCONFIG_LORA_FIXED_HEADER_LEN_MODE_DEFAULT=1
CFLAGS += -DLORA_PAYLOAD_CRC_ON_DEFAULT=1
CFLAGS += -DCONFIG_LORA_PAYLOAD_LENGTH_DEFAULT=10
Firmware Output
main(): This is RIOT! (Version: 2020.10-devel-1308-gae087-Kconfig_lora_tests)
CONFIG_LORA_PREAMBLE_LENGTH_DEFAULT=10
CONFIG_LORA_SYMBOL_TIMEOUT_DEFAULT=20
CONFIG_LORA_BW_DEFAULT=LORA_BW_250_KHZ
CONFIG_LORA_SF_DEFAULT=LORA_SF8
DCONFIG_LORA_CR_DEFAULT=LORA_CR_4_6
DCONFIG_LORA_IQ_INVERTED_DEFAULT=1
DCONFIG_LORA_FREQUENCY_HOPPING_DEFAULT=1
CONFIG_LORA_FREQUENCY_HOPPING_PERIOD_DEFAULT=10
CONFIG_LORA_FIXED_HEADER_LEN_MODE_DEFAULT=CONFIG_LORA_FIXED_HEADER_LEN_MODE_DEFAULT
CONFIG_LORA_PAYLOAD_CRC_OFF_DEFAULT=CONFIG_LORA_PAYLOAD_CRC_OFF_DEFAULT
CFLAGS += -DLORA_PAYLOAD_CRC_ON_DEFAULT=1
CFLAGS += -DCONFIG_LORA_PAYLOAD_LENGTH_DEFAULT=10

Usage with menuconfig :

make menuconfig

Default Firmware Output
main(): This is RIOT! (Version: 2020.10-devel-1308-gae087-Kconfig_lora_tests)
CONFIG_LORA_PREAMBLE_LENGTH_DEFAULT=8
CONFIG_LORA_SYMBOL_TIMEOUT_DEFAULT=10
CONFIG_LORA_BW_DEFAULT=(LORA_BW_125_KHZ)
CONFIG_LORA_SF_DEFAULT=(LORA_SF7)
CONFIG_LORA_CR_DEFAULT=(LORA_CR_4_5)
CONFIG_LORA_IQ_INVERTED_DEFAULT=CONFIG_LORA_IQ_INVERTED_DEFAULT
CONFIG_LORA_FREQUENCY_HOPPING_DEFAULT=CONFIG_LORA_FREQUENCY_HOPPING_DEFAULT
CONFIG_LORA_FREQUENCY_HOPPING_PERIOD_DEFAULT=(0U)
CONFIG_LORA_FIXED_HEADER_LEN_MODE_DEFAULT=CONFIG_LORA_FIXED_HEADER_LEN_MODE_DEFAULT
CONFIG_LORA_PAYLOAD_CRC_OFF_DEFAULT=CONFIG_LORA_PAYLOAD_CRC_OFF_DEFAULT
LORA_PAYLOAD_CRC_ON_DEFAULT=(true)
CONFIG_LORA_PAYLOAD_LENGTH_DEFAULT=10
Configured Firmware Output
main(): This is RIOT! (Version: 2020.10-devel-1308-gae087-Kconfig_lora_tests)
CONFIG_LORA_PREAMBLE_LENGTH_DEFAULT=10
CONFIG_LORA_SYMBOL_TIMEOUT_DEFAULT=15
CONFIG_LORA_BW_DEFAULT=(LORA_BW_250_KHZ)
CONFIG_LORA_SF_DEFAULT=(LORA_SF9)
CONFIG_LORA_CR_DEFAULT=(LORA_CR_4_6)
CONFIG_LORA_IQ_INVERTED_DEFAULT=1
CONFIG_LORA_FREQUENCY_HOPPING_DEFAULT=1
CONFIG_LORA_FREQUENCY_HOPPING_PERIOD_DEFAULT=100
CONFIG_LORA_FIXED_HEADER_LEN_MODE_DEFAULT=1
CONFIG_LORA_PAYLOAD_CRC_OFF_DEFAULT=1
LORA_PAYLOAD_CRC_ON_DEFAULT=(false)
CONFIG_LORA_PAYLOAD_LENGTH_DEFAULT=0

MACROS were successfully configured.

Issues/PRs references

#12888

@akshaim
Copy link
Member Author

akshaim commented Sep 11, 2020 •
edited
Loading

@jia200x Hey. Appreciate if you can suggest the unit of "LORA_FREQUENCY_HOPPING_PERIOD_DEFAULT". Not sure if it milliseconds or microseconds. The official documentation from Semtech for sx127x chips does not seem to have it.

@leandrolanzieri leandrolanzieri added Area: Kconfig Area: Kconfig integration Area: LoRa Area: LoRa radio support Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Sep 12, 2020
@leandrolanzieri leandrolanzieri mentioned this pull request Sep 12, 2020
Open
71 tasks
leandrolanzieri
leandrolanzieri reviewed Sep 15, 2020
View reviewed changes
sys/include/net/lora.h Outdated Show resolved Hide resolved
sys/include/net/lora.h Outdated Show resolved Hide resolved
@leandrolanzieri leandrolanzieri mentioned this pull request Sep 24, 2020
Merged
@jia200x
Copy link
Member

jia200x commented Sep 29, 2020

@jia200x Hey. Appreciate if you can suggest the unit of "LORA_FREQUENCY_HOPPING_PERIOD_DEFAULT". Not sure if it milliseconds or microseconds. The official documentation from Semtech for sx127x chips does not seem to have it.

The Hop Period is expressed in "symbols", not in a time unit :)

jia200x
jia200x reviewed Sep 30, 2020
View reviewed changes
sys/include/net/lora.h Outdated Show resolved Hide resolved
jia200x
jia200x reviewed Sep 30, 2020
View reviewed changes
sys/net/lora/Kconfig Outdated Show resolved Hide resolved
jia200x
jia200x reviewed Sep 30, 2020
View reviewed changes
sys/net/lora/Kconfig Outdated Show resolved Hide resolved
jia200x
jia200x reviewed Sep 30, 2020
View reviewed changes
sys/net/lora/Kconfig Outdated Show resolved Hide resolved
jia200x
jia200x reviewed Sep 30, 2020
View reviewed changes
sys/net/lora/Kconfig Outdated Show resolved Hide resolved
jia200x
jia200x reviewed Sep 30, 2020
View reviewed changes
sys/include/net/lora.h Outdated Show resolved Hide resolved
@akshaim
Copy link
Member Author

akshaim commented Sep 30, 2020 •
edited
Loading

I have removed all references to LoRaWAN to avoid confusion (if any)

jia200x
jia200x reviewed Sep 30, 2020
View reviewed changes
sys/include/net/lora.h Outdated Show resolved Hide resolved
jia200x
jia200x reviewed Sep 30, 2020
View reviewed changes
sys/include/net/lora.h Outdated Show resolved Hide resolved
@jia200x
Copy link
Member

jia200x commented Sep 30, 2020

I just added some last comment, but over all this looks ok. Please address those and also the doc issues pointed out by Travis-CI.

@jia200x
Copy link
Member

jia200x commented Sep 30, 2020

Looks good to me. Please squash

@jia200x
Copy link
Member

jia200x commented Sep 30, 2020

BTW you can directly amend the doc issues when you squash:

	ERROR: Doxygen generates the following warnings:

	deprecated:32: warning: unable to resolve reference to `CONFIG_LORA_PAYLOAD_CRC_OFF_DEFAULT' for \ref command

	sys/include/net/lora.h:180: warning: unable to resolve reference to `CONFIG_LORA_PAYLOAD_CRC_OFF_DEFAULT' for \ref

@akshaim
net/lora : Move 'LORA_PREAMBLE_LENGTH_DEFAULT' to 'CONFIG_'
de1ad1e
@akshaim
net/lora : Move 'LORA_SYMBOL_TIMEOUT_DEFAULT' to 'CONFIG_'
979700c
@akshaim
net/lora : Move 'LORA_BW_DEFAULT' to 'CONFIG_'
349d203 
Added choices and moved 'LORA_BW_DEFAULT" to 'CONFIG_' namespace
@akshaim
net/lora : Move 'LORA_SF_DEFAULT' to 'CONFIG_'
aaeed98
@akshaim
net/lora : Move 'LORA_CR_DEFAULT' to 'CONFIG_'
a161476 
Added choices and moved 'LORA_CR_DEFAULT' to 'CONFIG_' namespace
@akshaim
net/lora : Model as bool 'LORA_IQ_INVERTED_DEFAULT'
00cc04f 
Move 'LORA_IQ_INVERTED_DEFAULT' to 'CONFIG_'
@akshaim
net/lora : Model as bool 'LORA_FREQUENCY_HOPPING_DEFAULT'
acd601e 
Move 'LORA_FREQUENCY_HOPPING_DEFAULT' to 'CONFIG_'
@akshaim
net/lora : Move 'LORA_FREQUENCY_HOPPING_PERIOD_DEFAULT' to 'CONFIG_'
e329588
@akshaim
net/lora : Model as bool 'LORA_FIXED_HEADER_LEN_MODE_DEFAULT'
889828a 
Move 'LORA_FIXED_HEADER_LEN_MODE_DEFAULT' to 'CONFIG_'
@akshaim
net/lora : Mode as bool 'LORA_PAYLOAD_CRC_ON_DEFAULT'
1629b11 
Modelled 'LORA_PAYLOAD_CRC_ON_DEFAULT' as bool. Introduced
'CONFIG_LORA_PAYLOAD_CRC_OFF_DEFAULT' to invert logic and
tagged 'LORA_PAYLOAD_CRC_ON_DEFAULT' for deprecation
@akshaim
net/lora : Move 'LORA_PAYLOAD_LENGTH_DEFAULT' to 'CONFIG_'
399273d
@akshaim
net/lora : Expose to Kconfig
aa83367
@akshaim
net/lora : Remove unused macro 'LORA_FIX_LENGTH_PAYLOAD_ON_DEFAULT'
4dbbd93
@akshaim
net/lora : Update documentation
b57365c
@akshaim
net/lora : Change defaults
a9f7b88 
Change 'CONFIG_LORA_SF_DEFAULT' to 'SF7' and
'CONFIG_LORA_CR_DEFAULT' to 'LORA_CR_4_5' to
allow shorter air times and make it easier to meet country
specific regulations.
@akshaim
Copy link
Member Author

akshaim commented Sep 30, 2020

Looks good to me. Please squash

Here we go ☺️

@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 Sep 30, 2020
@jia200x
Copy link
Member

jia200x commented Sep 30, 2020

great! let's see what Murdock says

@leandrolanzieri leandrolanzieri 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 Oct 2, 2020
jia200x
jia200x approved these changes Oct 2, 2020
View reviewed changes
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

@jia200x jia200x merged commit f30da14 into RIOT-OS:master Oct 2, 2020
@jia200x
Copy link
Member

jia200x commented Oct 2, 2020

Thanks for your contribution!

@akshaim akshaim mentioned this pull request Nov 26, 2020
Merged
@akshaim akshaim deleted the Kconfig_lora branch December 2, 2020 15:52
@aabadie aabadie mentioned this pull request Jan 14, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leandrolanzieri leandrolanzieri leandrolanzieri left review comments

@jia200x jia200x jia200x approved these changes

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

@cgundogan cgundogan Awaiting requested review from cgundogan

@haukepetersen haukepetersen Awaiting requested review from haukepetersen

@miri64 miri64 Awaiting requested review from miri64

@PeterKietzmann PeterKietzmann Awaiting requested review from PeterKietzmann

Assignees
No one assigned
Labels
Area: Kconfig Area: Kconfig integration Area: LoRa Area: LoRa radio support CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
No milestone
Development

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