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

Kconfig: Expose pkg/wakaama configurations #12974

Merged
merged 6 commits into from
Apr 7, 2020
Merged

Kconfig: Expose pkg/wakaama configurations #12974

merged 6 commits into from
Apr 7, 2020

Conversation

leandrolanzieri
Copy link
Contributor

@leandrolanzieri leandrolanzieri commented Dec 17, 2019 •
edited
Loading

Contribution description

This PR moves the LwM2M Wakaama client configuration macros to the CONFIG_ namespace and exposes them to Kconfig.

Testing procedure

  • The Wakaama example should work as usual
  • You should be able to configure the client by running make menuconfig. Test e.g. changing the server URI and device name, enabling the package logs, and enabling the usage of a bootstrap server.

Issues/PRs references

Part of #12888
Depends on #12913

@leandrolanzieri leandrolanzieri added Area: network Area: Networking Type: new feature The issue requests / The PR implemements a new feature for RIOT State: waiting for other PR State: The PR requires another PR to be merged first TF: Config Marks issues and PRs related to the work of the Configuration Task Force labels Dec 17, 2019
@leandrolanzieri leandrolanzieri added this to the Release 2020.01 milestone Dec 17, 2019
@leandrolanzieri leandrolanzieri mentioned this pull request Dec 17, 2019
Open
71 tasks
@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/wakaama branch from 0564885 to dee81f1 Compare December 19, 2019 10:22
@pokgak pokgak mentioned this pull request Dec 19, 2019
Merged
@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/wakaama branch from dee81f1 to 28a7b98 Compare December 20, 2019 10:22
@leandrolanzieri
Copy link
Contributor Author

Rebased to master, this has no dependencies now.

@leandrolanzieri leandrolanzieri removed the State: waiting for other PR State: The PR requires another PR to be merged first label Dec 20, 2019
@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/wakaama branch from 28a7b98 to 1f299a5 Compare February 13, 2020 15:34
@leandrolanzieri leandrolanzieri added the Area: Kconfig Area: Kconfig integration label Feb 21, 2020
fjmolinas
fjmolinas reviewed Apr 7, 2020
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

First round of comments, also please rebase @leandrolanzieri. I'll test asap.

examples/wakaama/Makefile Outdated Show resolved Hide resolved
examples/wakaama/Makefile Show resolved Hide resolved
pkg/wakaama/include/lwm2m_client_config.h Show resolved Hide resolved
@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/wakaama branch from 1f299a5 to 3e4dfce Compare April 7, 2020 09:36
@fjmolinas
Copy link
Contributor

Symbol changes are good and tested configuration changes take effect:

image

@fjmolinas fjmolinas 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 Apr 7, 2020
@fjmolinas fjmolinas added the Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines label Apr 7, 2020
fjmolinas
fjmolinas reviewed Apr 7, 2020
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

I think documenation wise it would be good to add an a mention to the fact that this can all be set through kconfig/ make menuconfig

@leandrolanzieri
Copy link
Contributor Author

I think documenation wise it would be good to add an a mention to the fact that this can all be set through kconfig/ make menuconfig

I added some improvements to the README.md, also indicating how to bind the host on Leshan

@fjmolinas
Copy link
Contributor

@leandrolanzieri looks good now, please squash!

@fjmolinas fjmolinas added the Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines label Apr 7, 2020
fjmolinas
fjmolinas approved these changes Apr 7, 2020
View reviewed changes
Copy link
Contributor

@fjmolinas fjmolinas left a comment

Choose a reason for hiding this comment

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

ACK!, please trigger the CI once squashed!

@leandrolanzieri
pkg/wakaama: Move configurations to 'CONFIG_' namespace
c392a4a 
Macros that changed:
LWM2M_STANDARD_PORT -> CONFIG_LWM2M_STANDARD_PORT
LWM2M_DTLS_PORT -> CONFIG_LWM2M_DTLS_PORT
LWM2M_BSSERVER_PORT -> CONFIG_LWM2M_BSSERVER_PORT
LWM2M_LOCAL_PORT -> CONFIG_LWM2M_LOCAL_PORT
LWM2M_DEVICE_NAME -> CONFIG_LWM2M_DEVICE_NAME
LWM2M_DEVICE_TTL -> CONFIG_LWM2M_DEVICE_TTL
LWM2M_SERVER_URI -> CONFIG_LWM2M_SERVER_URI
LWM2M_SERVER_ID -> CONFIG_LWM2M_SERVER_ID
LWM2M_ALT_PATH -> CONFIG_LWM2M_ALT_PATH
LWM2M_BOOTSTRAP -> CONFIG_LWM2M_BOOTSTRAP
LWM2M_DEVICE_MANUFACTURER -> CONFIG_LWM2M_DEVICE_MANUFACTURER
LWM2M_DEVICE_MODEL -> CONFIG_LWM2M_DEVICE_MODEL
LWM2M_DEVICE_SERIAL -> CONFIG_LWM2M_DEVICE_SERIAL
LWM2M_DEVICE_FW_VERSION -> CONFIG_LWM2M_DEVICE_FW_VERSION
LWM2M_DEVICE_BINDINGS -> CONFIG_LWM2M_DEVICE_BINDINGS
LWM2M_DEVICE_TYPE -> CONFIG_LWM2M_DEVICE_TYPE
LWM2M_DEVICE_HW_VERSION -> CONFIG_LWM2M_DEVICE_HW_VERSION
LWM2M_DEVICE_SW_VERSION -> CONFIG_LWM2M_DEVICE_SW_VERSION
LWM2M_WITH_LOGS -> CONFIG_LWM2M_WITH_LOGS
@leandrolanzieri
examples/wakaama: Configure server only if not set by Kconfig
e2325dd
@leandrolanzieri
pkg/wakaama: Move client mode flag to package
d6b4cde 
As the only implementation available in RIOT of LwM2M is the client, it
makes no sense to set this flag in every application.
@leandrolanzieri
pkg/wakaama: Move LWM2M_TLSF_BUFFER to 'CONFIG_' namespace
a3f9b2b
@leandrolanzieri
Kconfig: Expose pkg/wakaama configurations
e5729ae
@leandrolanzieri
examples/wakaama: Improve README
ca17f0a
@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/wakaama branch from a770ac3 to ca17f0a Compare April 7, 2020 14:29
@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 7, 2020
@fjmolinas
Copy link
Contributor

GO!

@fjmolinas fjmolinas merged commit 7739b40 into RIOT-OS:master Apr 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fjmolinas fjmolinas fjmolinas approved these changes

@aabadie aabadie Awaiting requested review from aabadie

@kb2ma kb2ma Awaiting requested review from kb2ma

@cgundogan cgundogan Awaiting requested review from cgundogan

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

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 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 TF: Config Marks issues and PRs related to the work of the Configuration Task Force 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.
2 participants
@leandrolanzieri @fjmolinas