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

Fix building with modern PICO-SDK #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

intx82
Copy link

@intx82 intx82 commented Jan 11, 2025

Fix build mentioned in issue #16

@Plaque-fcc
Copy link

Hi! You suggest not building the demo blinky firmware intentionally or by an accident?

Copy link

@Plaque-fcc Plaque-fcc left a comment

Choose a reason for hiding this comment

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

Shouldn't really have other changes not mentioned in the name and/or not related to fixing the issue.

@intx82
Copy link
Author

intx82 commented Jan 14, 2025

There was an error in my case, so i just disable a blinky. As for me, it's redundant to build blinky while you not suggest use it.

And due to blinky have own shell script to build it instead of Makefile and i am using riscv32-unknown-elf-gcc instead of default riscv64-linux-elf-gcc (or whatever inside), it was too lazy to repair that i don't need. I think this option will be the same for the lot of people.

@Plaque-fcc
Copy link

There was an error in my case, so i just disable a blinky. As for me, it's redundant to build blinky while you not suggest use it.

And due to blinky have own shell script to build it instead of Makefile and i am using riscv32-unknown-elf-gcc instead of default riscv64-linux-elf-gcc (or whatever inside), it was too lazy to repair that i don't need. I think this option will be the same for the lot of people.

Thank you for your honest and elaborated answer. Although I don't agree with your style of submitting changes, I respect your decision.
Moreover, you helped me and others use the latest SDK version.

However, these two changes are not linked in any way, and only one of them is described in the commit message. Should be both. Or better separated in respective PRs each.

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.

2 participants