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

Apply this change to 2.0.9 #8131

Merged
merged 3 commits into from
May 3, 2023
Merged

Apply this change to 2.0.9 #8131

merged 3 commits into from
May 3, 2023

Conversation

SuGlider
Copy link
Collaborator

Description of Change

This change is missing in 2.0.9 (master).
Fixes F_CPU for all SoC.

Tests scenarios

Related links

#Fix #8005
#7913

devrim-oguz and others added 3 commits March 1, 2023 16:48
Hello, I was using the FastLED library and it was complaining about F_CPU not being defined. So, I just noticed that it is not defined for the ESP32-S3 module. So I made this change in the header file and it compiled. Therefore I wanted to propose this change to the HAL library to improve compatibility. Thank you for your time.
Works for ESP32, ESP32C3, ESP32S2, ESP32S3
Necessary for ESP32 Arduino Core 2.0.x based on IDF 4.4
@SuGlider SuGlider added this to the 2.0.9 milestone Apr 27, 2023
@SuGlider SuGlider requested a review from me-no-dev April 27, 2023 15:10
@SuGlider SuGlider self-assigned this Apr 27, 2023
@CLAassistant
Copy link

CLAassistant commented Apr 27, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ SuGlider
❌ devrim-oguz
You have signed the CLA already but the status is still pending? Let us recheck it.

@SuGlider
Copy link
Collaborator Author

@me-no-dev -
This PR should be also in 2.0.9, but not for 3.0.0 which has another PR for fixing it ( #8007 ).
It is already in release/2.x branch ( #7913 )
Not sure what is the right way to do it. Please advise.

@me-no-dev
Copy link
Member

@SuGlider if it's in release/v2.x then it will be in 2.0.9. No need to PR master, because it will go into 3.0.0 also that way.

@SuGlider
Copy link
Collaborator Author

@SuGlider if it's in release/v2.x then it will be in 2.0.9. No need to PR master, because it will go into 3.0.0 also that way.

It was already in release/v2.x and it didn't go to 2.0.8, when it was released...
That is the reason I think that it doesn't happen automatically.

@mrengineer7777
Copy link
Collaborator

@SuGlider you have an extra contributor that is blocking the license agreement

@me-no-dev
Copy link
Member

@SuGlider if it's in release/v2.x then it will be in 2.0.9. No need to PR master, because it will go into 3.0.0 also that way.

It was already in release/v2.x and it didn't go to 2.0.8, when it was released... That is the reason I think that it doesn't happen automatically.

I might have an idea of what went wrong... 2.0.9 will be fine

@VojtechBartoska
Copy link
Collaborator

VojtechBartoska commented May 3, 2023

@SuGlider can we change a PR title to something more specific? Right now, it's without any meaning for e.g. Release notes, thanks.

@mrengineer7777
Copy link
Collaborator

mrengineer7777 commented May 3, 2023

Suggested title: "Fix F_CPU for all SoCs".
If me-no-dev wants to merge this (likely not, see above), then suglider also needs to fix the contributor.

@me-no-dev me-no-dev merged commit 85d179c into espressif:master May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

esp32-s3 cannot find F_CPU for FastLed
6 participants