search

h2zero / esp-nimble-cpp

C++ library for the esp32 NimBLE stack based on and mostly compatible with @nkolban cpp_utils BLE library.
https://h2zero.github.io/esp-nimble-cpp/
Apache License 2.0
181 stars 62 forks source link

Fix check for arduino component #204

Closed jefflongo closed 1 month ago

jefflongo commented 1 month ago

Fixes #202. Tested with IDF 5.1.4 / Arduino core 3.0.x and IDF 4.4.8 / Arduino core 2.0.x.

jefflongo commented 1 month ago

As a side note, is __hack_component_targets really necessary? It doesn't seem necessary for Arduino, but I didn't remove it because I don't know for what cases it was added in the first place.

jefflongo commented 1 month ago

Also, the README states that defining ARDUINO_ARCH_ESP32 is required. Looking at my compile_commands.json, it seems to be defined without me defining it in the root CMakeLists.txt. Do you have any evidence that this is still required?

h2zero commented 1 month ago

Thanks for this, looks like the issue I had with this was a different problem. That being said, the reason the arduino dependency wasn't being added was simply due to the folder name. If you follow the instructions to use arduino as a component it clones the repo into the "arduino" folder, not "arduino-esp32", which is why this cmake file checks for that.

So, instead of replacing the check, we should check for both cases.

The __hack_component_targets is a bit of a legacy thing from years ago, probably not needed now but I'll review that soon.

You can ignore that bit in the readme, needs updating too.

jefflongo commented 1 month ago

The reason I missed that was because I followed the "Installing using IDF Component Manager" instruction in the Espressif documentation instead of the "Manual installation of Arduino framework" instruction. So you're right, we should be checking for both.