-
Notifications
You must be signed in to change notification settings - Fork 130
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
Add Thing Plus RP2040 Board #900
Conversation
08c289f
to
ab9deda
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
env.copy(repopath("tools/openocd/modm/rp2040_picoprobe.cfg"), "rp2040_picoprobe.cfg") | ||
env.collect(":build:openocd.source", "modm/board/rp2040_picoprobe.cfg") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about moving the programmer configuration into its own module, since it is not part of the actual devboard?
The RP2040-Picoprobe module can still be included in the blink example in project.xml
.
(This should of course also apply to all existing (STM32) Blue-Pill/Black-Pill/... Dev-Board without integrated programmer, but that's out of the scope of this pull request.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not super convinved, since it's just a single file, so seems weird to have it's own module for it.
Maybe it would make more if we had some "debug" module that's more separate from the build module, but that's kinda weird too.
If you write your own non-BSP based application, then you need to copy this file manually anyways?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always found it annoying in the past with boards like the Blue-Pill to have a different programmer (e.g. stlink v2 vs. stlink v3) than the one assumed in the BSP for no reason and then have to manually patch around in the generated code each time.
it's just a single file, so seems weird to have it's own module for it.
I do not see any disadvantage?
Probably the module should also add the logging functionality to the board (if the programmer has a integrated USB VCP channel) and then document the necessary wiring.
Maybe it would make more if we had some "debug" module that's more separate from the build module, but that's kinda weird too.
modm:debug
module exists, and contains modm::log::Logger
. Why is it called debug
?
If you write your own non-BSP based application, then you need to copy this file manually anyways?
Or just include the programmer module, if that is a module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have to manually patch around in the generated code each time.
Yeah, that's really annoying. But does specifying the programmer in the openocd.cfg
file not work as documented in the Blue Pill docs?
Probably the module should also add the logging functionality to the board (if the programmer has a integrated USB VCP channel) and then document the necessary wiring.
Ok, I like that.
modm:debug module exists, and contains modm::log::Logger. Why is it called debug?
Good old historic reasons of course. I would move it to modm:io:logger
.
Or just include the programmer module, if that is a module?
Sure, but if the BSP already includes the programmer module, then you cannot uninclude it again. So you're back to square one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not opposed to this at all btw, I think it's a really good idea to separate the programmer out, it just needs to be done well as per usual ;-P
ab9deda
to
f7bfdca
Compare
f7bfdca
to
874c8d6
Compare
No description provided.