search

espressif / arduino-esp32

Arduino core for the ESP32
GNU Lesser General Public License v2.1
12.97k stars 7.29k forks source link

BLE Codespace Reduction #7702

Open mrengineer7777 opened 1 year ago

mrengineer7777 commented 1 year ago

Related area

BLE

Hardware specification

ESP32

Is your feature request related to a problem?

I was browsing through the Arduino libraries, learning how xTaskCreate is used and noticed the BLE library includes https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/FreeRTOS.cpp and https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/RTOS.h. I believe this functionality already exists in the IDF and is duplicated here.

Describe the solution you'd like

Remove FreeRTOS.cpp and RTOS.h. Reference native functions instead.

Describe alternatives you've considered

Current code works but likely takes more space, and FreeRTOS definitions may diverge from IDF.

Additional context

No response

I have checked existing list of Feature requests and the Contribution Guide

SuGlider commented 1 year ago

Listing Related Files:

include"RTOS.H"

https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/BLEAdvertising.h#L15 https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/BLECharacteristic.h#L19 https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/BLERemoteCharacteristic.h#L21 https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/BLERemoteDescriptor.h#L18 https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/BLERemoteService.h#L18 https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/BLEScan.h#L18 https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/BLEServer.h#L23 https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/BLEService.h#L18 https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/GeneralUtils.cpp#L15

RTOS.H

https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/RTOS.h

include <freertos/FreeRTOS.h>

https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/BLEDevice.cpp#L9 https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/BLEUtils.cpp#L15

Copy of FreeRTOS.cpp

https://github.com/espressif/arduino-esp32/blob/master/libraries/BLE/src/FreeRTOS.cpp

SuGlider commented 1 year ago

@mrengineer7777 - Thanks for noticing and reporting it! I'll change it to use just one single FreeRTOS version.

mrengineer7777 commented 1 year ago

When removing BLE...FreeRTOS.cpp, you may also need to update https://github.com/espressif/arduino-esp32/blob/d342739308436e13e56a5c4d3fe597076a1d36b3/CMakeLists.txt#L167

me-no-dev commented 1 year ago

those files come from the original library that Neil Colban created. There might be some functionality that is needed there. Maybe just rename it to something more sane?

SuGlider commented 1 year ago

I'll compare the local RTOS.h and local FreeRTOS.cpp to the current IDF version. So far it seems to be just a namespace issue. I'll report here the conclusions.