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

Tier one common examples #529

Closed
wants to merge 4 commits into from

Conversation

TDHolmes
Copy link
Contributor

Summary

Unify common examples for tier one BSPs to reduce code duplication.

Checklist

  • CHANGELOG.md for the BSP or HAL updated
  • All new or modified code is well documented, especially public items
  • No new warnings or clippy suggestions have been introduced (see CI or check locally)
  • Add compilation of these common examples to CI

) -> (
bsp::hal::clock::GenericClockController,
bsp::RedLed,
UsbBusAllocator<UsbBus>,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to make these all Option<T>'s, but for some reason it didn't work properly when I tried. Need to figure this out especially if we'll be vending out things like Timer's and RTC's

Option<bsp::RedLed>,
Option<UsbBusAllocator<UsbBus>>,
) {
let mut clocks = bsp::hal::clock::GenericClockController::with_external_32kosc(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Might want to have an enum argument for external_32k/internal_32k/etc.

@sajattack
Copy link
Member

No offence, but this looks overly complicated which kind of defeats the purpose of the examples as a demonstration of how to get started with a board.

@TDHolmes
Copy link
Contributor Author

Fair criticism, I can see that perspective. I didn't think it was too bad but it might be enough to obscure things too much for people wanting to copy-paste get started on their project. Any other opinions?

@bradleyharden
Copy link
Contributor

It might work better if you have separate thumbv6m and thumbv7em examples. That seems to be driving a lot of the complexity. If you could create common examples, where the only difference is the definition of bsp, that would be ideal.

@bradleyharden
Copy link
Contributor

bradleyharden commented Oct 25, 2021

Also, I wouldn't try to abstract away the initialization code. Better to leave that in each example, even if it's redundant. It lets each example stand on its own, and it shows new users how to properly initialize.

Edit: Let me revise that. If you want to abstract some of the clocking, then I would do it in each individual BSP, and use a common name (e.g. configure_clocks). That way it becomes like any other common convenience function in the BSPs.

@TDHolmes
Copy link
Contributor Author

Thanks for the feedback. I think I'll close this for now and maybe revisit once clock::v2 lands. separate v6/v7 examples makes sense, that should eliminate the extra abstraction code I put in for this.

@TDHolmes TDHolmes closed this Oct 25, 2021
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.

3 participants