Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

include esp_attr.h where IRAM_ATTR is used #952

Merged
merged 2 commits into from
Feb 2, 2024
Merged

Conversation

remenyo
Copy link
Contributor

@remenyo remenyo commented Feb 1, 2024

These two commits contain a small fix in two files that allowed my PlatformIO project to build.

They reference IRAM_ATTR attribute, but do not not include esp_attr.h; now they do, with the same ifdef guard as the attributes themselves.

If this issue is not apparent for other people and only my system is wrong, feel free to close this request.

@jgromes jgromes merged commit 7945ffb into jgromes:master Feb 2, 2024
29 checks passed
@jgromes
Copy link
Owner

jgromes commented Feb 2, 2024

Thank you for the contribution - it's interesting this did not come up before, especially in ESP-IDF builds. I guess some headers are added automatically there, but I don't see any reason to not have this explicitly. Merged, thanks!

@arendst
Copy link

arendst commented Feb 25, 2024

There is no esp_attr.h on ESP8266 so compilation fails.

I see no issue on ESP32 removing this PR either...

@jgromes
Copy link
Owner

jgromes commented Feb 25, 2024

@arendst that is interesting - I tried building this for ESP8266 using Arduino core, and you are right, it's broken (https://github.com/jgromes/RadioLib/actions/runs/8039040028/job/21955616874)

However, it seems that the esp_attr.h file does exist in ESP8266 SDK (https://github.com/espressif/ESP8266_RTOS_SDK/blob/master/components/esp8266/include/esp_attr.h), just not in its Arduino core, which is very strange, especially since it does exist in Arduino core for ESP32.

I changed the macro to check for defined(ESP_PLATFORM), which is set for ESP8266 SDK and ESP-IDF, but not set in Arduino ESP8266 core. It now compiles for ESP8266 and ESP32 Arduino cores as well as for ESP-IDF and for ESP32 in Platformio.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants