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

Move CYW43-related PICO_CONFIG lines to cyw43_driver.h #2002

Merged
merged 1 commit into from
Nov 9, 2024

Conversation

lurch
Copy link
Contributor

@lurch lurch commented Oct 27, 2024

This is a follow-up to #1982 - I'm afraid that I didn't think to check at the time, but the tools/extract_configs.py script was getting upset that there were duplicate PICO_CONFIG entries in different files. So this PR fixes that.

kilograham
kilograham previously approved these changes Nov 4, 2024
@kilograham kilograham added this to the 2.0.1 milestone Nov 4, 2024
Copy link
Contributor

@peterharperuk peterharperuk left a comment

Choose a reason for hiding this comment

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

Sorry, I don't like this. Why are we putting Pico specific GPIO configuration in src/rp2_common/pico_cyw43_driver/include/pico/cyw43_driver.h?

@lurch
Copy link
Contributor Author

lurch commented Nov 4, 2024

Sorry, I don't like this. Why are we putting Pico specific GPIO configuration in src/rp2_common/pico_cyw43_driver/include/pico/cyw43_driver.h?

I'm happy to take out the Pico-specific values from cyw43_driver.h but that'd also mean that the PICO_CONFIG lines no longer have a default= value. Let me know if you'd prefer that and I'll update the PR.

@peterharperuk
Copy link
Contributor

I don't really like these defines in cyw43_driver.h. I'd rather remove the PICO_CONFIG lines completely if they're causing a problem. Although this sounds like a tooling defect to me.

@lurch
Copy link
Contributor Author

lurch commented Nov 5, 2024

I've just force-pushed an updated commit, which removes the defines from cyw43_driver.h

@peterharperuk
Copy link
Contributor

I've just force-pushed an updated commit, which removes the defines from cyw43_driver.h

ta

@kilograham kilograham merged commit f642b76 into develop Nov 9, 2024
6 checks passed
@kilograham kilograham deleted the move_cyw43_pico_configs branch November 9, 2024 01:04
lurch added a commit that referenced this pull request Nov 11, 2024
 - Re-synchronise the PICO_CMAKE_CONFIG: entries with the corresponding PICO_CONFIG: entries
 - Fix a silly typo I made in #2002
 - Enhance config-extraction scripts to catch similar typos in future
@lurch lurch mentioned this pull request Nov 11, 2024
lurch added a commit that referenced this pull request Nov 12, 2024
 - Re-synchronise the PICO_CMAKE_CONFIG: entries with the corresponding PICO_CONFIG: entries
 - Fix a silly typo I made in #2002
 - Enhance config-extraction scripts to catch similar typos in future
kilograham pushed a commit that referenced this pull request Nov 19, 2024
- Re-synchronise the PICO_CMAKE_CONFIG: entries with the corresponding PICO_CONFIG: entries
 - Fix a silly typo I made in #2002
 - Enhance config-extraction scripts to catch similar typos in future
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants