rp-rs / rp-hal-boards

Board Support Packages for RP2040 based PCBs
199 stars 82 forks source link

Add support for Sparkfun XRP #45

Closed ThadHouse closed 10 months ago

ThadHouse commented 11 months ago

Adds support for https://www.sparkfun.com/products/22230 Copied mostly from the sparkfun-pro-micro

jannic commented 10 months ago

I just merged https://github.com/rp-rs/rp-hal-boards/pull/44, migrating the individual Cargo.toml files to use workspace dependencies. Could you change the Cargo.toml you added for the Sparkfun XPR accordingly?

ThadHouse commented 10 months ago

@jannic Updated both this and my other 2 PR's as well.

ThadHouse commented 10 months ago

Can the examples be done in a later PR? Doing examples for this specifically are kind of invasive and I don't think should block at least the merging of this.

ThadHouse commented 10 months ago

The main thing to figure out for the examples is many of them need special functionality. Like the quadrature encoders are a PIO program. That would make sense as a library, but not clear yet if it makes sense to go into the hal, have a new library with each individual thing (This smells way too JS like and I hate it), put it in the BSP, or have it just be a part of the examples.

ithinuel commented 10 months ago

I initiated a conversation on matrix to gather other people's thoughts.

I don't have strong opinions about the pins for wireless not being used yet because of lack of wifi support.

We are all volunteers with other day jobs. When we accept a crate (a BSP here, or any crate under rp-rs' org), we also (implicitly?) commit to maintaining it and keeping it current with the HAL's and other dependencies' updates. So the issue I see is that accepting more and more BSPs increases the work load the community has to go through for updates/releases, dutsing it off from time to time.

With this in mind, I'd just like to make sure that when we accept to take care of/"own" a crate, it must at least bring something new/different. Otherwise people may as-well use an existing crate with the same features but slightly different names. This is only to "safe-guard" our time and ensure it brings a bit more value to the community.

Others may disagree, and I'm fine to have this merged if they do.

ithinuel commented 10 months ago

[…] Like the quadrature encoders are a PIO program. That would make sense as a library, but not clear yet if it makes sense to go into the hal, have a new library with each individual […]

This is indeed how PIO features are usually exposed (see i2c-pio, spi-pio, ws2812-pio).

put it in the BSP, or have it just be a part of the examples.

If this is tiny and can fit in 10-20lines, that would be fine IMHO.

If quadrature encoder are too much work, there's still:

I haven't looked at the details of those drivers but they don't need to be feature complete to be used in an example. If at some point a user wants to update them to showcase a more feature complete driver that's fine too :)

ThadHouse commented 10 months ago

I think this is actually one BSP my team would probably want to have our own repo for. Theres a lot of functionality on this board and its probably better to have it be standalone.

jannic commented 10 months ago

We could still add a link to that bsp to the readme if you like.