net/coap: Expose configurations to Kconfig by leandrolanzieri · Pull Request #13227 · RIOT-OS/RIOT

leandrolanzieri · 2020-01-29T09:26:27Z

Contribution description

This moves the configuration macros for our generic CoAP options to the CONFIG_ namespace, and exposes them to Kconfig.

I didn't include a range constraint on COAP_ACK_TIMEOUT and COAP_RANDOM_FACTOR_1000 yet as I am not sure which would be a valid maximum. According to RFC 7252, section 4.8.1:

In particular, a decrease of ACK_TIMEOUT below 1 second would violate the guidelines of [RFC5405].

(So the minimum for COAP_ACK_TIMEOUT would be 1)

ACK_RANDOM_FACTOR MUST NOT be decreased below 1.0, and it SHOULD have a value that is sufficiently different from 1.0 to provide some protection from synchronization effects.

(So the minimum for COAP_RANDOM_FACTOR_1000 would be 1000)

Also, I made use of a 'feature' symbol (HAS_PROTOCOL_COAP) which indicates in a generic way that there is a module that provides this, as this options are generic. ~~When nanocoap has its own Kconfig file then it should be selected from there.~~

Edit: If #13243 is merged first, the TODO in the Kconfig file should be removed.

Testing procedure

Build examples/gcoap. By default everything should still work the same.
Modify some parameters (e.g. the number of retries or the random factor), changes should be applied.

Issues/PRs references

Part of #12888
Related to #13243

leandrolanzieri · 2020-03-13T11:33:05Z

Rebased to resolve conflicts.

miri64

I didn't include a range constraint on COAP_ACK_TIMEOUT and COAP_RANDOM_FACTOR_1000 yet as I am not sure which would be a valid maximum. […]

Maybe @kb2ma can help here?

Also, I made use of a 'feature' symbol (HAS_PROTOCOL_COAP) which indicates in a generic way that there is a module that provides this, as this options are generic. When nanocoap has its own Kconfig file then it should be selected from there.

Not really sure what you mean by that? Does this mean that nanocoap and gcoap would become mutually exclusive? oO

leandrolanzieri · 2020-04-03T15:28:50Z

Not really sure what you mean by that? Does this mean that nanocoap and gcoap would become mutually exclusive? oO

No, this does not do that. It just indicates that there is CoAP implementation using this options.

The reasoning was, that these options are defined 'generic CoAP options' for RIOT, so I did not want to make them depend on nanocoap being present. The HAS_PROTOCOL_COAP symbol is a generic way of indicating "hey, there are CoAP capabilities". If the symbol is y, then we show the options, and we can configure common CoAP options for whatever implementation that is used.

kb2ma · 2020-04-07T16:00:18Z

I didn't include a range constraint on COAP_ACK_TIMEOUT and COAP_RANDOM_FACTOR_1000 yet as I am not sure which would be a valid maximum. […]

Maybe @kb2ma can help here?

Sorry, no recommendation for max values. I have not had occasion to change them myself.

miri64 · 2020-04-08T09:05:03Z

Please rebase!

leandrolanzieri · 2020-04-08T09:11:36Z

Rebased and removed the TODO, as #13243 has been merged.

bergzand · 2020-04-21T10:11:57Z

sys/net/application_layer/Kconfig.coap

+        @ref CONFIG_COAP_ACK_TIMEOUT and
+        (@ref CONFIG_COAP_ACK_TIMEOUT * @ref CONFIG_COAP_RANDOM_FACTOR_1000 / 1000).


The doxygen @ref do not work with kconfig, maybe simply omit them in the help here.

I know Kconfig files are not generating documentation yet, but I plan to do so in the future. So this is just so we don't lose track of references that are currently on the docs.

Is there a way to add the marker so that it doesn't show up in the kconfig help?

Leaving a note that this is being addressed in #13916

See #13916

Didn't see that 😄

bergzand · 2020-04-21T10:12:53Z

Does it make sense to add a note to the help descriptions that the default values are copied from the RFC?

leandrolanzieri · 2020-04-21T10:37:52Z

Does it make sense to add a note to the help descriptions that the default values are copied from the RFC?

Yes. I added a reference to https://tools.ietf.org/html/rfc7252#section-4.8 now.

bergzand · 2020-04-24T07:53:57Z

Please Squash

COAP_ACK_TIMEOUT is moved to the compile time configuration macro namespace.

COAP_RANDOM_FACTOR_1000 is moved to the compile time configuration macro namespace.

COAP_MAX_RETRANSMIT is moved to the compile time configuration macro namespace.

leandrolanzieri · 2020-04-24T08:11:14Z

Please Squash

Squashed

bergzand

Ack

leandrolanzieri · 2020-04-24T08:48:38Z

Thanks for reviewing !

leandrolanzieri added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: CoAP Area: Constrained Application Protocol implementations TF: Config Marks issues and PRs related to the work of the Configuration Task Force labels Jan 29, 2020

leandrolanzieri added this to the Release 2020.04 milestone Jan 29, 2020

leandrolanzieri requested review from cgundogan and kb2ma January 29, 2020 09:26

This was referenced Jan 29, 2020

[CTF-tracker] Kconfig: Configuration exposure list #12888

Open

net/nanocoap: Expose configurations to Kconfig #13243

Merged

leandrolanzieri added the Area: Kconfig Area: Kconfig integration label Feb 21, 2020

leandrolanzieri force-pushed the pr/kconfig_migrate/net/coap branch from 4f5089a to 04f3e9a Compare March 13, 2020 11:24

leandrolanzieri requested review from PeterKietzmann, bergzand, fjmolinas, haukepetersen, jia200x, kaspar030 and miri64 as code owners March 13, 2020 11:24

leandrolanzieri force-pushed the pr/kconfig_migrate/net/coap branch from 04f3e9a to 02ac496 Compare March 13, 2020 11:28

leandrolanzieri force-pushed the pr/kconfig_migrate/net/coap branch from 02ac496 to f912723 Compare April 3, 2020 08:46

miri64 reviewed Apr 3, 2020

View reviewed changes

leandrolanzieri force-pushed the pr/kconfig_migrate/net/coap branch from f912723 to a03c9c1 Compare April 8, 2020 09:10

leandrolanzieri modified the milestones: Release 2020.04, Release 2020.07 Apr 10, 2020

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 16, 2020

cgundogan removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 16, 2020

cgundogan 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 Apr 16, 2020

bergzand reviewed Apr 21, 2020

View reviewed changes

leandrolanzieri mentioned this pull request Apr 21, 2020

tools/kconfiglib: Add riot_kconfig to override default behaviours #13916

Merged

leandrolanzieri added 4 commits April 24, 2020 10:06

net/coap: Move COAP_ACK_TIMEOUT to 'CONFIG' namespace

072a0d8

COAP_ACK_TIMEOUT is moved to the compile time configuration macro namespace.

net/coap: Move COAP_RANDOM_FACTOR_1000 to 'CONFIG' namespace

afe4244

COAP_RANDOM_FACTOR_1000 is moved to the compile time configuration macro namespace.

net/coap: Move COAP_MAX_RETRANSMIT to 'CONFIG' namespace

9e427e1

COAP_MAX_RETRANSMIT is moved to the compile time configuration macro namespace.

net/coap: Expose configurations to Kconfig

eeea9b7

leandrolanzieri force-pushed the pr/kconfig_migrate/net/coap branch from 58232ee to eeea9b7 Compare April 24, 2020 08:10

bergzand approved these changes Apr 24, 2020

View reviewed changes

bergzand merged commit 268152b into RIOT-OS:master Apr 24, 2020

		@ref CONFIG_COAP_ACK_TIMEOUT and
		(@ref CONFIG_COAP_ACK_TIMEOUT * @ref CONFIG_COAP_RANDOM_FACTOR_1000 / 1000).

Conversation

leandrolanzieri commented Jan 29, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

leandrolanzieri commented Mar 13, 2020

Uh oh!

miri64 left a comment

Choose a reason for hiding this comment

Uh oh!

leandrolanzieri commented Apr 3, 2020

Uh oh!

kb2ma commented Apr 7, 2020

Uh oh!

miri64 commented Apr 8, 2020

Uh oh!

leandrolanzieri commented Apr 8, 2020

Uh oh!

bergzand Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

leandrolanzieri Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

bergzand Apr 21, 2020

Choose a reason for hiding this comment

Uh oh!

bergzand Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

leandrolanzieri Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

leandrolanzieri Apr 24, 2020

Choose a reason for hiding this comment

Uh oh!

bergzand commented Apr 21, 2020

Uh oh!

leandrolanzieri commented Apr 21, 2020

Uh oh!

bergzand commented Apr 24, 2020

Uh oh!

leandrolanzieri commented Apr 24, 2020

Uh oh!

bergzand left a comment

Choose a reason for hiding this comment

Uh oh!

leandrolanzieri commented Apr 24, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

leandrolanzieri commented Jan 29, 2020 •

edited

Loading