Kconfig: Expose Gcoap configurations by leandrolanzieri · Pull Request #12887 · RIOT-OS/RIOT

leandrolanzieri · 2019-12-06T08:45:40Z

Contribution description

This PR adds the CONFIG_ prefix to Gcoap's configuration macros and then exposes those options to Kconfig.

The approach is to have a menuconfig entry for the module (KCONFIG_MODULE_GCOAP) which enables/disables the configuration of Gcoap via Kconfig. This entry will only show when the module is being used (depends on MODULE_GCOAP).

MODULE_ symbols are declared in the Kconfig.dep file generated from USEMODULES:

RIOT/makefiles/kconfig.mk

Lines 54 to 58 in c6b9f95

    
           $(KCONFIG_GENERATED_DEPENDENCIES): FORCE | $(GENERATED_DIR) 
        
           	$(Q)printf "%s " $(USEMODULE) \ 
        
           	  | awk 'BEGIN {RS=" "}{ gsub("-", "_", $$0); \ 
        
           	      printf "config MODULE_%s\n\tbool\n\tdefault y\n", toupper($$0)}' \ 
        
           	  | $(LAZYSPONGE) $(LAZYSPONGE_FLAGS) $@

Testing procedure

Gcoap's example application should still be working as usual (make all). In this case the applied configurations should be the ones declared in gcoap.h.
Running make menuconfig in Gcoap's example application should reveal the menuconfig interface and Gcoap's configurations. Configurations selected via this interface should be applied to the application.

Issues/PRs references

#12888

Kconfig

leandrolanzieri · 2019-12-11T09:33:21Z

Rebased to master, 'system' and 'net' Kconfig files are merged now.

tcschmidt · 2020-01-24T11:59:18Z

ping @leandrolanzieri @cgundogan

cgundogan

On a side note: I really like this interface. It is very simple to discover available configurations :) Much better than searching for the correct header file or doxygen page, IMO ..

There seems to be something off here. I tested the gcoap example:

make all

works perfectly.

make menuconfig

I change the port and save the config. Another make all gives now:

 % make
Building application "gcoap_example" for "native" with MCU "native".

In file included from <command-line>:
RIOT/examples/gcoap/bin/native/generated/autoconf.h:82: error: "CONFIG_GCOAP_RESEND_BUFS_MAX" redefined [-Werror]
   82 | #define CONFIG_GCOAP_RESEND_BUFS_MAX 1
      | 
<command-line>: note: this is the location of the previous definition
In file included from <command-line>:
RIOT/examples/gcoap/bin/native/riotbuild/riotbuild.h:2: error: "CONFIG_GCOAP_RESEND_BUFS_MAX" redefined [-Werror]
    2 | #define CONFIG_GCOAP_RESEND_BUFS_MAX 2
      | 
In file included from <command-line>:
RIOT/examples/gcoap/bin/native/generated/autoconf.h:82: note: this is the location of the previous definition
   82 | #define CONFIG_GCOAP_RESEND_BUFS_MAX 1
      | 
cc1: all warnings being treated as errors
make[1]: *** [RIOT/Makefile.base:100: RIOT/examples/gcoap/bin/native/application_gcoap_example/gcoap_cli.o] Error 1
make: *** [RIOT/examples/gcoap/../../Makefile.include:515: RIOT/examples/gcoap/bin/native/application_gcoap_example.a] Error 2

Any ideas what might be wrong?

leandrolanzieri · 2020-01-27T08:08:15Z

On a side note: I really like this interface. It is very simple to discover available configurations :) Much better than searching for the correct header file or doxygen page, IMO ..

👍

Any ideas what might be wrong?

Yes, as this was one of the first PRs we still didn't have the Kconfig symbols available in Makefiles, so it is expected to have errors if configurations are set via CFLAGS. I will update this to check for Kconfig usage and avoid conflicts.

sys/net/application_layer/gcoap/Kconfig

leandrolanzieri · 2020-01-27T08:30:44Z

@cgundogan I rebased so I can use the change of #12913

cgundogan

I retested after the rebase. I could rebuild after make menuconfig without any errors. I also confirmed that changes done via kconfig propagate to the binary: I changed the GCOAP_PORT in the menu and could interact with the resource endpoint using the new port.

cgundogan

Tested the fixup commit: observable tick length ranges now from 1..3 and kconfig does not allow values outside this range.

ACK. Please squash!

Macros that changed: GCOAP_MSG_QUEUE_SIZE -> CONFIG_GCOAP_MSG_QUEUE_SIZE GCOAP_NO_AUTO_INIT -> CONFIG_GCOAP_NO_AUTO_INIT GCOAP_NO_RETRANS_BACKOFF -> CONFIG_GCOAP_NO_RETRANS_BACKOFF GCOAP_NON_TIMEOUT -> CONFIG_GCOAP_NON_TIMEOUT GCOAP_OBS_CLIENTS_MAX -> CONFIG_GCOAP_OBS_CLIENTS_MAX GCOAP_OBS_OPTIONS_BUF -> CONFIG_GCOAP_OBS_OPTIONS_BUF GCOAP_OBS_REGISTRATIONS_MAX -> CONFIG_GCOAP_OBS_REGISTRATIONS_MAX GCOAP_OBS_VALUE_WIDTH -> CONFIG_GCOAP_OBS_VALUE_WIDTH GCOAP_PDU_BUF_SIZE -> CONFIG_GCOAP_PDU_BUF_SIZE GCOAP_PORT -> CONFIG_GCOAP_PORT GCOAP_RECV_TIMEOUT -> CONFIG_GCOAP_RECV_TIMEOUT GCOAP_REQ_OPTIONS_BUF -> CONFIG_GCOAP_REQ_OPTIONS_BUF GCOAP_REQ_WAITING_MAX -> CONFIG_GCOAP_REQ_WAITING_MAX GCOAP_RESEND_BUFS_MAX -> CONFIG_GCOAP_RESEND_BUFS_MAX GCOAP_RESP_OPTIONS_BUF -> CONFIG_GCOAP_RESP_OPTIONS_BUF GCOAP_TOKENLEN -> CONFIG_GCOAP_TOKENLEN

This adds a check in the Makefile so configurations are set via CFLAGS only if Kconfig is not being used as the configurator for the module. Otherwise there may be a conflict.

leandrolanzieri · 2020-01-28T11:01:01Z

Tested the fixup commit: observable tick length ranges now from 1..3 and kconfig does not allow values outside this range.

ACK. Please squash!

@cgundogan squashed

kaspar030 · 2020-01-28T11:16:37Z

sys/net/application_layer/gcoap/Kconfig

+menu "Timeouts and retries"
+
+config GCOAP_RECV_TIMEOUT
+    int "Incoming message timeout"


general question on KConfig: are these converted to explicitly typed literals (1000000LU)?

No, Kconfig does not add suffixes to these literals, the header file will just contain 1000000. If needed, could we add a cast when using the macro?

So all type checking by the compiler is removed?

So all type checking by the compiler is removed?

no, the compiler checks the type. What is removed is the possibility to set types for integer literals. This only affects CONFIG symbols that are used in arithmetic expressions. Also, the compiler still throws an error if integer widths do not match. Using an explicit cast (...) solves this problem.

@kaspar030 ready to merge?

Murdock shows green lights. Let's merge this now, but we can still continue the discussion in this thread for this particular kconfig property, if needed.

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 Dec 6, 2019

leandrolanzieri requested review from kb2ma, miri64 and smlng December 6, 2019 08:45

leandrolanzieri mentioned this pull request Dec 6, 2019

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

Open

71 tasks

cgundogan reviewed Dec 6, 2019

View reviewed changes

Kconfig Show resolved Hide resolved

leandrolanzieri added this to the Release 2020.01 milestone Dec 6, 2019

leandrolanzieri force-pushed the pr/kconfig_migrate/gcoap branch from 2cfb09d to c941996 Compare December 11, 2019 09:31

fjmolinas modified the milestones: Release 2020.01, Release 2020.04 Jan 16, 2020

cgundogan requested changes Jan 24, 2020

View reviewed changes

leandrolanzieri commented Jan 27, 2020

View reviewed changes

sys/net/application_layer/gcoap/Kconfig Show resolved Hide resolved

leandrolanzieri force-pushed the pr/kconfig_migrate/gcoap branch from c941996 to cc4ab70 Compare January 27, 2020 08:29

cgundogan approved these changes Jan 27, 2020

View reviewed changes

cgundogan approved these changes Jan 28, 2020

View reviewed changes

leandrolanzieri added 3 commits January 28, 2020 11:59

Kconfig: Expose Gcoap configurations

926d32f

examples/gcoap: Set configs via CFLAGS if Kconfig is not used

cf7a738

This adds a check in the Makefile so configurations are set via CFLAGS only if Kconfig is not being used as the configurator for the module. Otherwise there may be a conflict.

leandrolanzieri force-pushed the pr/kconfig_migrate/gcoap branch from ad75697 to cf7a738 Compare January 28, 2020 11:00

kaspar030 reviewed Jan 28, 2020

View reviewed changes

cgundogan merged commit c84d7d2 into RIOT-OS:master Jan 28, 2020

leandrolanzieri deleted the pr/kconfig_migrate/gcoap branch January 28, 2020 18:19

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

	$(KCONFIG_GENERATED_DEPENDENCIES): FORCE \| $(GENERATED_DIR)
	$(Q)printf "%s " $(USEMODULE) \
	\| awk 'BEGIN {RS=" "}{ gsub("-", "_", $$0); \
	printf "config MODULE_%s\n\tbool\n\tdefault y\n", toupper($$0)}' \
	\| $(LAZYSPONGE) $(LAZYSPONGE_FLAGS) $@

Conversation

leandrolanzieri commented Dec 6, 2019 • 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!

Uh oh!

leandrolanzieri commented Dec 11, 2019

Uh oh!

tcschmidt commented Jan 24, 2020

Uh oh!

cgundogan left a comment

Choose a reason for hiding this comment

Uh oh!

leandrolanzieri commented Jan 27, 2020

Uh oh!

Uh oh!

leandrolanzieri commented Jan 27, 2020

Uh oh!

cgundogan left a comment

Choose a reason for hiding this comment

Uh oh!

cgundogan left a comment

Choose a reason for hiding this comment

Uh oh!

leandrolanzieri commented Jan 28, 2020

Uh oh!

kaspar030 Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

leandrolanzieri Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

miri64 Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

cgundogan Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

cgundogan Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

cgundogan Jan 28, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

leandrolanzieri commented Dec 6, 2019 •

edited

Loading