manuelbl / ttn-esp32

The Things Network device library for ESP32 (ESP-IDF) and SX127x based devices
MIT License
301 stars 64 forks source link

Fix include errors #47

Closed mkfrey closed 2 years ago

mkfrey commented 2 years ago

When compiling as component of ESP-IDF v4.3.2 I got the following compilation errors:

$(pwd)/components/ttn-esp32/src/hal/hal_esp32.c:119:5: error: implicit declaration of function 'gpio_pad_select_gpio'; did you mean 'esp_rom_gpio_pad_select_gpio'? [-Werror=implicit-function-declaration]
     gpio_pad_select_gpio(pin_nss);
     ^~~~~~~~~~~~~~~~~~~~
     esp_rom_gpio_pad_select_gpio
$(pwd)/components/ttn-esp32/src/aes/mbedtls_aes.c:13:10: fatal error: mbedtls/aes.h: No such file or directory
 #include "mbedtls/aes.h"
          ^~~~~~~~~~~~~~~

This PR fixes both of them.

manuelbl commented 2 years ago

Thanks for the PR. Both issues should be fixed.

However, I cannot reproduce the problem, with neither ESP-IDF 4.3.2 nor with 4.4. Did this occur with one of the sample projects such as hello_world? Did you use any special settings?

mkfrey commented 2 years ago

I am using custom code without any special settings. But I've changed so much of it today that I'm no longer able to reproduce the issue as well. But I know that a full clean didn't solve the problem. Maybe I accidentally used esp-idf master.

Nonetheless, driver/gpio.h already contains a warning stating that the inclusion of rom/gpio.h will be removed on next release and that esp_rom_gpio.h is the way to go forward.

manuelbl commented 2 years ago

I agree that it needs to be changed.

However instead of replacing gpio_pad_select_gpio() with the even more low-level esp_rom_gpio_pad_select_gpio() function, I will replace it with gpio_reset_pin(), pending some testing.

mkfrey commented 2 years ago

Sounds like a good idea. I was already wondering why such a low level call that I've personally never encountered is even needed.

esp_rom_gpio_pad_select_gpio() is btw. just an alias to gpio_pad_select_gpio(), so I'ts not really more low level. I think espressif just wants to make it more clear that this isn't an ordinary gpio_* function.

mkfrey commented 2 years ago

I've checked again, and it looks like I was accidentally using esp-idf master. So my commits would fix the build on master.

manuelbl commented 2 years ago

Thanks a lot. I've merged your PR with the additional discussed changes.