[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #1893: xtensa/esp32: Add ESP32 WiFi adapter and driver

[GitBox <gi...@apache.org>]

donghengqaz commented on a change in pull request #1893:
URL: https://github.com/apache/incubator-nuttx/pull/1893#discussion_r494716868



##########
File path: arch/xtensa/src/esp32/Make.defs
##########
@@ -136,3 +136,36 @@ ifeq ($(CONFIG_ARCH_USE_MODULE_TEXT),y)
 CHIP_CSRCS += esp32_modtext.c
 CMN_ASRCS += xtensa_loadstore.S
 endif
+
+ifeq ($(CONFIG_ESP32_WIRELESS),y)
+WIRELESS_DRV_UNPACK  = esp-wireless-drivers-3rdparty
+WIRELESS_DRV_ZIP     = master.zip
+WIRELESS_DRV_URL     = https://github.com/espressif/esp-wireless-drivers-3rdparty/archive

Review comment:
       > I only added comments to some simple things because I cannot really review the actual contents of this PR since I'm not familiar with WiFi code in NuttX. However, I must say that the adapter file has a **lot** going on, it is a really big file with lots of auxiliary functions. Also, I feel that is quite an obscure implementation, it is very difficult to understand what is this code doing in general, I only get little pieces by looking at the small comments for each function.
   > 
   > It would be good to add some general documentation, either in the form of a README or simple as more detailed comments explaining what all this code is doing and what each part is doing.
   > I fear that besides the obvious fact that there's some closed code behind this, this code itself will be difficult to maintain/debug by someone else as is.
   > 
   > That said, I really appreciate the support for ESP32 WiFi on NuttX.
   
   There are 2 main source files, one is `esp32_wlan.c` and another is `esp32_wifi_adapter.c`, we can think the `esp32_wlan.c` is alike with `esp32_emac.c`, and it realizes the Ethernet communication. `esp32_wifi_adapter.c` is mainly contains of `OS adapter` fucntions, like `task, semaphore, mutex, message queue`, even including `libc`, `log`, `file system` and so on. So it mainly have no specific logic. 
   
   But when espressif developed the wifi driver, they don't fully consider porting this driver to other OS platforms, so there are some complex designing which I think should be taken into libraries. Because these logic functions are not related to porting OS adapter functions. So I have been trying to improve the wifi adapter layer to reduce the functions which we should add to support other OS.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org