Skip to content

Conversation

chrysn
Copy link
Member

@chrysn chrysn commented Feb 22, 2024

Contribution description

Some vendor files we have in the ESP CPU area were modified to pick up our defines, in particular MCU_ESP8266. Our MCU went away in #20397, this fixes things by doing a change in the style of what was already done there for the ESP CPUs. (My original checks left those out because those were vendor files, and most other vendors have all kinds of MCU_SOMETHING_INTERNAL defines that are unrelated to our MCU defines).

Testing procedure

  • ESP8266 comes up with any applicaiton (I don't have one, reviewers please check)

Issues/PRs references

Follow-up-for: #20397 (where I broke this)
Closes: #20409 (which inspired this)
Closes: #20415 (which would also solve the issue but also keep the MCU_ variable mess)

Acknowledgements

Thanks @maribu for finding in #20498 the precise cause, I'm just offering a different solution.

@chrysn chrysn requested a review from gschorcht as a code owner February 22, 2024 13:30
@github-actions github-actions bot added Platform: ESP Platform: This PR/issue effects ESP-based platforms Area: cpu Area: CPU/MCU ports labels Feb 22, 2024
@chrysn chrysn added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer labels Feb 22, 2024
@benpicco benpicco requested a review from maribu February 22, 2024 13:32
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution also works and does so without CFLAGS trickery, so let's go with that.

@chrysn chrysn enabled auto-merge February 22, 2024 13:36
@riot-ci
Copy link

riot-ci commented Feb 22, 2024

Murdock results

✔️ PASSED

c3020ce cpu/esp: Use CPU_ESP8266 define instead of the removed MCU_ESP8266

Success Failures Total Runtime
9997 0 9997 08m:42s

Artifacts

@chrysn chrysn added this pull request to the merge queue Feb 22, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 22, 2024
@chrysn chrysn added this pull request to the merge queue Feb 22, 2024
Merged via the queue into RIOT-OS:master with commit 6e892d9 Feb 22, 2024
@chrysn chrysn deleted the alt20415 branch February 22, 2024 20:47
@MrKevinWeiss MrKevinWeiss added this to the Release 2024.04 milestone Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants