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/tinydtls configurations #12992

Merged
merged 3 commits into from
Mar 6, 2020
Merged

Kconfig: Expose pkg/tinydtls configurations #12992

merged 3 commits into from
Mar 6, 2020

Conversation

pokgak
Copy link
Contributor

@pokgak pokgak commented Dec 19, 2019 •
edited
Loading

Contribution description

This PR moves the tinydtls configuration macros to the CONFIG_ namespace and exposes them to Kconfig.

Testing procedure

  • dtls-echo, dtls-sock with tinydtls should work as usual
  • You should be able to configure the client by running make menuconfig. Test e.g. changing cipher suites, enable debug log.

Test each configuration option, e.g. set CONFIG_DTLS_DEBUG through:

  • make menuconfig and enable/disable debug log
  • Pass CFLAGS=CONFIG_DTLS_DEBUG to make when building the application
  • Neither of the above. Use the default value set by the application

The example applications should build as usual and use the configured option values.

Issues/PRs references

Part of #12888
Depends on #12913, #12974

@leandrolanzieri leandrolanzieri mentioned this pull request Dec 19, 2019
Open
71 tasks
@pokgak pokgak force-pushed the pr/kconfig_migrate/tinydtls branch 2 times, most recently from e589a24 to 49aed1d Compare December 19, 2019 16:19
@leandrolanzieri leandrolanzieri added Area: pkg Area: External package ports 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 labels Dec 20, 2019
@leandrolanzieri leandrolanzieri added this to the Release 2020.01 milestone Dec 20, 2019
leandrolanzieri
leandrolanzieri reviewed Jan 2, 2020
View reviewed changes
Copy link
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.

Here are some comments. We need to come up with a solution for CFLAGS being used in the application's Makefile.include. As it is right now e.g. dtls-sock will always set CONFIG_DTLS_PSK even when Kconfig is being used.

pkg/tinydtls/Makefile.include Outdated Show resolved Hide resolved
examples/dtls-echo/README.md Outdated Show resolved Hide resolved
examples/dtls-echo/README.md Outdated Show resolved Hide resolved
@fjmolinas
Copy link
Contributor

ping @pokgak!

@pokgak
Copy link
Contributor Author

pokgak commented Jan 15, 2020

Added backwards compability for using DTLS_PSK, DTLS_ECC, DTLS_DEBUG and deprecation note until release 2020.10 in 829a9f1.

leandrolanzieri
leandrolanzieri reviewed Jan 20, 2020
View reviewed changes
pkg/tinydtls/Makefile.include Outdated Show resolved Hide resolved
pkg/tinydtls/Makefile.include Outdated Show resolved Hide resolved
pkg/tinydtls/Kconfig Outdated Show resolved Hide resolved
pkg/tinydtls/Makefile.include Outdated Show resolved Hide resolved
pkg/tinydtls/Makefile.include Outdated Show resolved Hide resolved
@pokgak
Copy link
Contributor Author

pokgak commented Feb 5, 2020

Sorry for the late response. I addresed your comments in the latest commits.

@pokgak
Copy link
Contributor Author

pokgak commented Feb 6, 2020 •
edited
Loading

Are your comments resolved @leandrolanzieri? If its okay, I'm gonna squash and rebase to master to pull in #13290 changes to fix the reported branch conflict.

leandrolanzieri
leandrolanzieri reviewed Feb 7, 2020
View reviewed changes
Copy link
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.

Sorry for the delay. Yes, please address this one comment, rebase and squash. I will begin testing when you do.

pkg/tinydtls/Makefile.include Outdated Show resolved Hide resolved
@pokgak pokgak force-pushed the pr/kconfig_migrate/tinydtls branch from 465ad17 to 0c6a98c Compare February 7, 2020 09:28
@pokgak
Copy link
Contributor Author

pokgak commented Feb 7, 2020

Squashed and rebased.

@leandrolanzieri leandrolanzieri added the Area: Kconfig Area: Kconfig integration label Feb 20, 2020
leandrolanzieri
leandrolanzieri approved these changes Mar 5, 2020
View reviewed changes
Copy link
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.

Tested and everything still works as usual. Configurations via Kconfig are applied correctly. ACK.

@leandrolanzieri
Copy link
Contributor

@pokgak please rebase

@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 Mar 5, 2020
@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 CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 5, 2020
leandrolanzieri and others added 2 commits March 5, 2020 19:11
@leandrolanzieri
Kconfig: Add 'Packages' file and menu
7a44d8e
pkg/tinydtls: Move configurations to 'CONFIG_' namespace
d5b2aa8 
Macros that changed:
DTLS_PSK -> CONFIG_DTLS_PSK
DTLS_ECC -> CONFIG_DTLS_ECC (except in release-notes.txt)
DTLS_CONTEXT_MAX -> CONFIG_DTLS_CONTEXT_MAX
DTLS_PEER_MAX -> CONFIG_DTLS_PEER_MAX
DTLS_HANDSHAKE_MAX -> CONFIG_DTLS_HANDSHAKE_MAX
DTLS_SECURITY_MAX -> CONFIG_DTLS_SECURITY_MAX
DTLS_HASH_MAX -> CONFIG_DTLS_HASH_MAX
@pokgak pokgak force-pushed the pr/kconfig_migrate/tinydtls branch from 0c6a98c to 817a7c5 Compare March 5, 2020 18:11
@pokgak
Copy link
Contributor Author

pokgak commented Mar 5, 2020

Rebased to current master.

leandrolanzieri
leandrolanzieri reviewed Mar 5, 2020
View reviewed changes
Kconfig Outdated Show resolved Hide resolved
Kconfig Outdated Show resolved Hide resolved
@pokgak pokgak force-pushed the pr/kconfig_migrate/tinydtls branch from 817a7c5 to 05d8341 Compare March 5, 2020 18:35
@pokgak pokgak force-pushed the pr/kconfig_migrate/tinydtls branch from 05d8341 to a6432cf Compare March 6, 2020 12:54
@leandrolanzieri
Copy link
Contributor

And GO!

@leandrolanzieri leandrolanzieri merged commit 1ee1a4f into RIOT-OS:master Mar 6, 2020
@aabadie aabadie mentioned this pull request Oct 30, 2020
Merged
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

@aabadie aabadie Awaiting requested review from aabadie

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

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

Assignees
No one assigned
Labels
Area: Kconfig Area: Kconfig integration Area: pkg Area: External package ports 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.
3 participants
@pokgak @fjmolinas @leandrolanzieri