[riot-notifications] [RIOT-OS/RIOT] cpu/esp32: esp_wifi netdev driver (#10762)

Sebastian Meiling notifications at github.com
Wed Jan 16 09:17:46 CET 2019


smlng commented on this pull request.

code wise this looks pretty much okay and ready to go, however we really need some testing on this one: @MrKevinWeiss and @MichelRottleuthner can you give it quick try, our working group AP should distribute an IPv6 ULA prefix (`fd..` something) so you can use that for testing.

> +in module `esp_idf_wpa_supplicant_crypto`, which is required by module
+`esp_wifi`, `esp_wifi` cannot be used for applications that use these modules.
+Therefore, module `esp_wifi` is not automatically enabled when module
+`netdev_default` is used. Instead, if necessary, the application has to add
+the module `esp_wifi` in the Makefile.
+
+```
+USEMODULE += esp_wifi
+```
+
+Furthermore, the following configuration parameters have to be defined:
+
+Configuration Parameter | Description
+------------------------|------------
+ESP_WIFI_SSID           | SSID of the AP to be used.
+ESP_WIFI_PASS           | Passphrase used for the AP (max. 64 chars).

clear text or hashed?

> @@ -74,12 +74,22 @@ esp_err_t _esp_wifi_rx_cb(void *buffer, uint16_t len, void *eb)
 {
     DEBUG("%s: buf=%p len=%d eb=%p\n", __func__, buffer, len, eb);
 
-    CHECK_PARAM_RET (buffer != NULL, -EINVAL);
-    CHECK_PARAM_RET (len <= ETHERNET_DATA_LEN, -EINVAL);
+    if (buffer == NULL || len >= ETHERNET_DATA_LEN) {

minor nit: please enclose conditions in parentheses

> @@ -74,12 +74,22 @@ esp_err_t _esp_wifi_rx_cb(void *buffer, uint16_t len, void *eb)
 {
     DEBUG("%s: buf=%p len=%d eb=%p\n", __func__, buffer, len, eb);
 
-    CHECK_PARAM_RET (buffer != NULL, -EINVAL);
-    CHECK_PARAM_RET (len <= ETHERNET_DATA_LEN, -EINVAL);
+    if (buffer == NULL || len >= ETHERNET_DATA_LEN) {
+        if (eb) {

mhm a bit of inconsistent code usage/style, i.e. above you've `buffer == NULL` which could be `!buffer` or this one should be `eb != NULL` 😄 

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/10762#pullrequestreview-193010893
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190116/b63a0fba/attachment.html>


More information about the notifications mailing list