cpu/esp*: add Kconfig #13135

gschorcht · 2020-01-15T16:35:29Z

Contribution description

This PR adds Kconfig for ESP32 and ESP8266 CPUs as well as the ESP netdev drivers esp_wifi
and esp_now.

Testing procedure

Use examples/gnrc_networking with commands

USEMODULE='esp_wifi esp_now' make BOARD=esp32-wroom-32 -C examples/gnrc_networking menuconfig
USEMODULE='esp_wifi esp_now' make BOARD=esp8266-esp-12x -C examples/gnrc_networking menuconfig

to configure both CPUs and try to compile them afterwards.

Issues/PRs references

Part of issue #12888

gschorcht · 2020-01-15T20:11:00Z

Just force-pushed typo and whitespace fixes for static tests.

leandrolanzieri

Some initial comments. Regarding the macro renaming: I think it's better if we start to keep the CONFIG_ namespace for macros generated via Kconfig, that way at a glance you can tell where this is coming from.

Also, take a look at this regarding the translation of choices:

RIOT/sys/usb/Kconfig

Lines 30 to 43 in 27764fe

    
           choice 
        
               bool "USB specification version" 
        
           config USB_SPEC_BCDVERSION_2_0 
        
               bool "USB v2.0" 
        
               help 
        
                   The peripheral acts as an USB version 2.0 device. 
        
           config USB_SPEC_BCDVERSION_1_1 
        
               bool "USB v1.1" 
        
               help 
        
                   The peripheral acts as an USB version 1.1 device. 
        
           endchoice

RIOT/sys/include/usb.h

Lines 94 to 102 in 27764fe

    
           #ifndef CONFIG_USB_SPEC_BCDVERSION 
        
           #if defined(CONFIG_USB_SPEC_BCDVERSION_1_1) 
        
           #define CONFIG_USB_SPEC_BCDVERSION      0x0110 
        
           #elif defined(CONFIG_USB_SPEC_BCDVERSION_2_0) 
        
           #define CONFIG_USB_SPEC_BCDVERSION      0x0200 
        
           #else 
        
           #define CONFIG_USB_SPEC_BCDVERSION      0x0200 
        
           #endif 
        
           #endif

Lastly, I know that these are CPU-specific configurations for the ESP, but having the WiFi and ESP-NOW options under CPU seems a bit off from the user perspective when using menuconfig.

cpu/esp32/Kconfig

leandrolanzieri · 2020-01-16T09:27:53Z

cpu/esp_common/esp-now/esp_now_params.h

@@ -21,6 +21,64 @@

 #if defined(MODULE_ESP_NOW) || defined(DOXYGEN)

+/**
+ * @name    Legacy definitions of default configuration parameters
+ * @{


Maybe add the @deprecated note and a deprecation release (2020.10?)

leandrolanzieri · 2020-01-16T09:30:34Z

cpu/esp_common/esp-now/Kconfig

+    bool "Configure ESP-NOW netdev"
+    depends on MODULE_ESP_NOW
+    help
+        Configure ESP-NOW netdev when module esp_now is used.


This only shows up when esp_now module is used, so there is no need to clarify that on the help string.

leandrolanzieri · 2020-01-16T09:42:29Z

cpu/esp_common/esp-now/esp_now_params.h

@@ -30,42 +88,42 @@
 * @brief   The size of the stack used for the ESP-NOW netdev driver thread.
 * @ingroup cpu_esp_common_conf
 */
-#ifndef ESP_NOW_STACKSIZE
-#define ESP_NOW_STACKSIZE       (THREAD_STACKSIZE_DEFAULT)
+#ifndef CONFIG_ESP_NOW_STACKSIZE


This macro is not generated by Kconfig, I think it's better that we try to keep the CONFIG_ namespace for those

I decided to rename it for two reasons:

I wished the stack size would be configurable, but I couldn't find an usable approach til now. The option has to be an int which doesn't allow a default like THREAD_STACKSIZE_DEFAULT. Maybe, I will add a combination of a bool option for using the default and an int option for specifying a different value.

I wanted to avoid naming inconsistencies from the users point of view in documentations where you have tables like

Configuration Parameter Description

CONFIG_ESP_WIFI_SSID SSID of the AP to be used.

CONFIG_ESP_WIFI_PASS Passphrase used for the AP as clear text

CONFIG_ESP_WIFI_STACKSIZE Stack size used for the WiFi netdev driver

I wanted to avoid tables like the following:

Configuration Parameter Description

CONFIG_ESP_WIFI_SSID SSID of the AP to be used.

CONFIG_ESP_WIFI_PASS Passphrase used for the AP as clear text

ESP_WIFI_STACKSIZE Stack size used for the WiFi netdev driver

Therefore, I decided to name everything that is configurable by user also at command line in same way.

leandrolanzieri · 2020-01-16T09:43:51Z

cpu/esp_common/esp-now/esp_now_params.h

-#ifndef ESP_NOW_KEY
-#define ESP_NOW_KEY             (NULL)
+#ifndef CONFIG_ESP_NOW_KEY
+#define CONFIG_ESP_NOW_KEY          ESP_NOW_KEY


The configuration of the key with Kconfig is a problem. The option would have to be either 0 or a 64 character array. I have no idea how to realize it with Kconfig. I renamed it to CONFIG_ just for naming consistency.

leandrolanzieri · 2020-01-16T09:59:08Z

cpu/esp8266/esp-wifi/Kconfig

@@ -0,0 +1,29 @@
+# Copyright (c) 2020 Gunar Schorcht


Is there a reason to have the exact same Kconfig for esp8266 and esp32 in different places, instead of having one in esp_common like the ESP-NOW?

Yes, when RIOT was ported to both platforms, the ESP8266 SDK required an implementation of esp_wifi that was completely different from that for ESP32.

With the new ESP8266 SDK and the complete reimplementation of the RIOT port on ESP8266 in PR #11108, a new esp_wifi module was introduced for ESP8266 that is almost the same as for ESP32. That's why there is already PR #12955 which will do a complete code deduplication. Module esp_wifi will then be moved to esp_common completely.

But before PR #12955 is merged, we have to do it in both implementations 😟 I wished PR #12955 would be merged soon, doing everything twice is quite nerving.

But, we can just place this file in cpu/esp_common/Kconfig.wifi and source it from both right?

But, we can just place this file in cpu/esp_common/Kconfig.wifi and source it from both right?

Yes, this should be possible.

leandrolanzieri · 2020-01-16T09:59:49Z

cpu/esp8266/esp-wifi/esp_wifi_params.h

-#ifndef ESP_WIFI_STACKSIZE
-#define ESP_WIFI_STACKSIZE  (THREAD_STACKSIZE_DEFAULT)
+#ifndef CONFIG_ESP_WIFI_STACKSIZE
+#define CONFIG_ESP_WIFI_STACKSIZE   ESP_WIFI_STACKSIZE


leandrolanzieri · 2020-01-16T09:59:56Z

cpu/esp8266/esp-wifi/esp_wifi_params.h

-#ifndef ESP_WIFI_PRIO
-#define ESP_WIFI_PRIO       (GNRC_NETIF_PRIO)
+#ifndef CONFIG_ESP_WIFI_PRIO
+#define CONFIG_ESP_WIFI_PRIO        ESP_WIFI_PRIO


leandrolanzieri · 2020-01-16T10:00:42Z

cpu/esp32/esp-wifi/esp_wifi_params.h

-#ifndef ESP_WIFI_STACKSIZE
-#define ESP_WIFI_STACKSIZE  (THREAD_STACKSIZE_DEFAULT)
+#ifndef CONFIG_ESP_WIFI_STACKSIZE
+#define CONFIG_ESP_WIFI_STACKSIZE   ESP_WIFI_STACKSIZE


leandrolanzieri · 2020-01-16T10:00:49Z

cpu/esp32/esp-wifi/esp_wifi_params.h

-#ifndef ESP_WIFI_PRIO
-#define ESP_WIFI_PRIO       (GNRC_NETIF_PRIO)
+#ifndef CONFIG_ESP_WIFI_PRIO
+#define CONFIG_ESP_WIFI_PRIO        ESP_WIFI_PRIO


gschorcht · 2020-01-16T13:42:53Z

Lastly, I know that these are CPU-specific configurations for the ESP, but having the WiFi and ESP-NOW options under CPU seems a bit off from the user perspective when using menuconfig.

What whould be a better place from your point of view?

leandrolanzieri · 2020-01-16T14:25:32Z

What whould be a better place from your point of view?

I would expect to see this maybe in System > Networking > WiFi.

One thing that can be done is to have generic 'WiFi' symbols (e.g. CONFIG_WIFI_SSID) that can be translated to esp-specific parameters. Something similar like LoRaMAC has now with the keys

gschorcht · 2020-01-16T14:33:51Z

What whould be a better place from your point of view?

I would expect to see this maybe in System > Networking > WiFi.

Hm 🤔 ok. Maybe there should be a further menu Network Devices and then WiFi and ESP-NOW?

`System > Networking > Network Devices > WiFi`
`System > Networking > Network Devices > ESP-NOW`

One thing that can be done is to have generic 'WiFi' symbols (e.g. CONFIG_WIFI_SSID)

As long a there is no other WiFi netdev driver I would leave the symbols as they are. OK?

leandrolanzieri · 2020-01-16T14:44:43Z

As long a there is no other WiFi netdev driver I would leave the symbols as they are. OK?

The problem with having the ESP specific symbols is that then you need to source a file that is placed in cpu/... from a file that is in sys/net/..., which may seem odd as we are trying to follow the same structure as we have for modules.

gschorcht · 2020-01-16T14:56:12Z

As long a there is no other WiFi netdev driver I would leave the symbols as they are. OK?

The problem with having the ESP specific symbols is that then you need to source a file that is placed in cpu/... from a file that is in sys/net/..., which may seem odd as we are trying to follow the same structure as we have for modules.

Yeah, that was the reason why I placed them in CPU section. Having ESP-WiFi in Networking and ESP-NOW in CPU section would be even more odd because ESP-NOW is nothing else than the WiFi interface that uses IEEE 802.11 Action Frames. So if we place ESPs network devices in Networking, the user would expect all of them there.

gschorcht · 2020-01-16T15:10:52Z

Something similar like LoRaMAC has now with the keys

I couldn't find Kconfig for LoRaMAC. I found sys/net/gnrc/link_layer/lorawan/Kconfig, but in this example all symbols are also called GNRC_LORAWAN_*.

leandrolanzieri · 2020-01-17T10:06:10Z

I couldn't find Kconfig for LoRaMAC.

Sorry, I was just referring to the fact that they have generic LORAMAC_ macros that are used both by the Semtech package and GNRC LoRaWAN.

leandrolanzieri · 2020-01-17T10:13:38Z

cpu/esp32/Kconfig

+
+if KCONFIG_CPU_ESP32
+
+choice ESP32_DEFAULT_CPU_FREQ_MHZ


There is no need to associate a symbol to the choice here, as it is not extended somewhere else.

leandrolanzieri · 2020-01-17T10:30:38Z

cpu/esp8266/Kconfig

+
+if KCONFIG_CPU_ESP8266
+
+choice ESP8266_CPU_FREQUENCY


leandrolanzieri · 2020-01-17T10:44:58Z

As long a there is no other WiFi netdev driver I would leave the symbols as they are. OK?

The problem with having the ESP specific symbols is that then you need to source a file that is placed in cpu/... from a file that is in sys/net/..., which may seem odd as we are trying to follow the same structure as we have for modules.

Yeah, that was the reason why I placed them in CPU section. Having ESP-WiFi in Networking and ESP-NOW in CPU section would be even more odd because ESP-NOW is nothing else than the WiFi interface that uses IEEE 802.11 Action Frames. So if we place ESPs network devices in Networking, the user would expect all of them there.

We could place ESP-NOW there as well

leandrolanzieri · 2020-04-03T09:23:43Z

Thus it's time to change the ESP-specific ESP_WIFI_* configurations to more general WIFI_* definitions. We should change it first before we expose the configuration in Kconfig.

That's good, and would be more in the direction of one of my proposals above, in order to have the WiFi parameters in a more generic place.

Ok, I'll be tagging this for next release then. Thanks for the quick response!

miri64 · 2020-06-25T13:51:24Z

@leandrolanzieri @gschorcht still valid?

gschorcht · 2020-06-25T13:55:57Z

Because of PR #13754, it is time to change the ESP specific WiFi configuration to a more general one. This must be done first. Then we can expose the configurations via Kconfig. I would say, that it nothing we can realize with this release.

fjmolinas · 2020-09-11T07:14:23Z

#13754 is in now, maybe @leandrolanzieri can help rework this one now

leandrolanzieri · 2020-10-22T07:18:34Z

@gschorcht any plans on moving forward with this one?

gschorcht · 2020-10-22T16:24:56Z

@gschorcht any plans on moving forward with this one?

Unfortunately not, I could not work on my RIOT contributions in the last months, because of other tasks related to the Corona online summer semester and the upcoming winter semester, which will be online again 😟 There are a number of open RIOT contributions just waiting to be completed. But I just don't have time to finish them and contribute at the moment.

fjmolinas · 2021-11-18T09:38:18Z

@leandrolanzieri @gschorcht esp is one of the main items on the to-do list for Kconfig, is it interesting to revive this PR?

leandrolanzieri · 2021-11-18T09:43:24Z

@leandrolanzieri @gschorcht esp is one of the main items on the to-do list for Kconfig, is it interesting to revive this PR?

Sure, I'm currently working on the modelling of the esp modules (minus networking), adding the configurations as well would be nice.

gschorcht · 2021-11-18T10:57:55Z

Sure, I'm currently working on the modelling of the esp modules (minus networking)

@leandrolanzieri Is your modelling esp modules something different or will you provide a new PR which makes this one obsolete?

leandrolanzieri · 2021-11-18T11:02:04Z

Sure, I'm currently working on the modelling of the esp modules (minus networking)

@leandrolanzieri Is your modelling esp modules something different or will you provide a new PR which makes this one obsolete?

It is something different, I will be providing a different PR (similar to #16929), but it does not make this one obsolete, as this adds configurations, not modules.

gschorcht · 2021-11-18T11:21:12Z

@leandrolanzieri Is your modelling esp modules something different or will you provide a new PR which makes this one obsolete?

It is something different, I will be providing a different PR (similar to #16929), but it does not make this one obsolete, as this adds configurations, not modules.

So my first task would be to rebase this PR.

leandrolanzieri · 2021-11-18T16:39:31Z

It is something different, I will be providing a different PR

@gschorcht see #17232

gschorcht · 2021-12-11T13:21:58Z

@leandrolanzieri Now were PR #17232 is merged, I would like to resume my work on the Kconfig for ESP network devices. I think this PR has to be reworked completely. Therefore, I would like to clarify the following questions before

We have now a category Drivers -> Network Device Drivers where there are already network device drivers. I would add ESP network devices there?
I would like to leave the configuration parameters like ESP_WIFI_* as documented to keep the compatibility with existing applications at user side. I would use the more general symbols CONFIG_WIFI_* in Kconfig and would map them as follows:
```
#ifndef ESP_WIFI_SSID
#ifdef CONFIG_WIFI_SSID
#define ESP_WIFI_SSID    CONFIG_WIFI_SSID
#else
#define ESP_WIFI_SSID    "RIOT_AP"
#endif
#endif
```
Would that be ok?
How can I enable GNRC in Kconfig for tests? I have seen in

RIOT/sys/net/gnrc/Kconfig

Lines 7 to 8 in 85313cc

menu "GNRC Network stack"

depends on USEMODULE_GNRC

that Kconfig for GNRC depends on USEMODULE_GNRC, but how can I enable it an required submodules? According to the documentation USEMODULE_ is generated by Kconfig from USEMODULE += gnrc. But it works neither at command line nor in Makefiles.

stale · 2022-07-10T11:12:07Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

cpu: add Kconfig section for all CPUs

1588fa9

leandrolanzieri mentioned this pull request Jan 15, 2020

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

Open

71 tasks

leandrolanzieri self-requested a review January 15, 2020 17:03

gschorcht added 5 commits January 15, 2020 21:09

cpu/esp_common: add Kconfig for module esp_now

5d40b07

cpu/esp8266: add Kconfig for CPU configuration

57a29da

cpu/esp32: add Kconfig for CPU configuration

d64257b

cpu/esp8266: add Kconfig for module esp_wifi

9d51cb5

cpu/esp32: add Kconfig for module esp_wifi

d51872e

gschorcht force-pushed the cpu/esp/kconfig branch from 82ddc73 to d51872e Compare January 15, 2020 20:10

leandrolanzieri requested changes Jan 16, 2020

View reviewed changes

gschorcht added 2 commits January 16, 2020 15:47

fixup! cpu/esp8266: add Kconfig for CPU configuration

3b1fb70

fixup! cpu/esp32: add Kconfig for CPU configuration

946ce9b

fjmolinas 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 Jan 17, 2020

leandrolanzieri reviewed Jan 17, 2020

View reviewed changes

leandrolanzieri added this to the Release 2020.07 milestone Apr 3, 2020

leandrolanzieri added the Area: Kconfig Area: Kconfig integration label Apr 3, 2020

gschorcht modified the milestones: Release 2020.07, Release 2020.10 Jun 25, 2020

gschorcht mentioned this pull request Jun 26, 2020

drivers: add ATWINC15x0 WiFi netdev driver #13754

Merged

jia200x modified the milestones: Release 2020.10, Release 2021.01, Release 2021.04 Jan 14, 2021

kaspar030 modified the milestones: Release 2021.04, Release 2021.07 Apr 23, 2021

MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021

gschorcht mentioned this pull request Dec 12, 2021

drivers/atwinc15x0: add Kconfig #17382

Open

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Jul 10, 2022

stale bot closed this Aug 13, 2022

	choice
	bool "USB specification version"

	config USB_SPEC_BCDVERSION_2_0
	bool "USB v2.0"
	help
	The peripheral acts as an USB version 2.0 device.

	config USB_SPEC_BCDVERSION_1_1
	bool "USB v1.1"
	help
	The peripheral acts as an USB version 1.1 device.

	endchoice

	#ifndef CONFIG_USB_SPEC_BCDVERSION
	#if defined(CONFIG_USB_SPEC_BCDVERSION_1_1)
	#define CONFIG_USB_SPEC_BCDVERSION 0x0110
	#elif defined(CONFIG_USB_SPEC_BCDVERSION_2_0)
	#define CONFIG_USB_SPEC_BCDVERSION 0x0200
	#else
	#define CONFIG_USB_SPEC_BCDVERSION 0x0200
	#endif
	#endif

Configuration Parameter	Description
CONFIG_ESP_WIFI_SSID	SSID of the AP to be used.
CONFIG_ESP_WIFI_PASS	Passphrase used for the AP as clear text
CONFIG_ESP_WIFI_STACKSIZE	Stack size used for the WiFi netdev driver

cpu/esp*: add Kconfig #13135

cpu/esp*: add Kconfig #13135

Conversation

gschorcht commented Jan 15, 2020

Contribution description

Testing procedure

Issues/PRs references

gschorcht commented Jan 15, 2020

leandrolanzieri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gschorcht Jan 16, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gschorcht commented Jan 16, 2020

leandrolanzieri commented Jan 16, 2020

gschorcht commented Jan 16, 2020

leandrolanzieri commented Jan 16, 2020

gschorcht commented Jan 16, 2020

gschorcht commented Jan 16, 2020

leandrolanzieri commented Jan 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leandrolanzieri commented Jan 17, 2020 • edited Loading

leandrolanzieri commented Apr 3, 2020

miri64 commented Jun 25, 2020

gschorcht commented Jun 25, 2020 • edited Loading

fjmolinas commented Sep 11, 2020

leandrolanzieri commented Oct 22, 2020

gschorcht commented Oct 22, 2020

fjmolinas commented Nov 18, 2021

leandrolanzieri commented Nov 18, 2021

gschorcht commented Nov 18, 2021

leandrolanzieri commented Nov 18, 2021

gschorcht commented Nov 18, 2021

leandrolanzieri commented Nov 18, 2021

gschorcht commented Dec 11, 2021 • edited Loading

stale bot commented Jul 10, 2022

gschorcht Jan 16, 2020 •

edited

Loading

leandrolanzieri commented Jan 17, 2020 •

edited

Loading

gschorcht commented Jun 25, 2020 •

edited

Loading

gschorcht commented Dec 11, 2021 •

edited

Loading