Skip to content

Swap to USB pad trait for thumv7em targets #365

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

Merged
merged 6 commits into from
Jan 25, 2021

Conversation

ryankurte
Copy link
Contributor

Supersedes #362, related to #363

An alternative approach that:

  • Introduces UsbPadD{m,p} marker traits for implementation over USB pads
  • Implements UsbPadD{m, p} marker trait over GPIO v1 and v2 types
  • Adds <Dm, Dp> generic arguments to UsbBus and other types that default to existing (v1) pad types

This allows USB pad types to be overridden with something like:

// Override default USB GPIO types to v2
pub type UsbBus = atsamd_hal::usb::UsbBus<Pin<gpio::PA24, Alternate<H>>, Pin<gpio::PA25, Alternate<H>>>;

While (hopefully) maintaining compatibility with previous users of UsbBus and v1 GPIOs.

Copy link
Contributor

@twitchyliquid64 twitchyliquid64 left a comment

Choose a reason for hiding this comment

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

Awesomesauce!

@twitchyliquid64
Copy link
Contributor

I thought you looked familiar! I was at Kawaiicon last year + distantly work with Grace's team! 😀

Small world ^^

@ryankurte
Copy link
Contributor Author

hah, wild how small it is sometimes ^_^

@twitchyliquid64
Copy link
Contributor

(WRT the PR, LGTM but as mentioned in #363 (comment) I'll wait till @bradleyharden has had a chance to look at it)

@bradleyharden
Copy link
Contributor

@ryankurte,

From #362:

I couldn't find an .into() bound that would work between v1 and v2, i might have just missed it though?

I added an implementation of From in both directions between v1 and v2, but it missed the cut for the v0.11 release. @twitchyliquid64 just released v0.12, which now includes it. Instead of your current approach, you could make the new function generic with bounds like Dm: Into<Pin<PA24, AlternateH>>. That should be backwards compatible and accept both versions.

If you want to stick with a new trait for other reasons, you might want to make crate::typelevel::Sealed a super trait. That way, users won't be able to implement your new trait themselves, even if they try.

@twitchyliquid64
Copy link
Contributor

The GPIO trait changes are merged in - is this ready to go @ryankurte / @bradleyharden or should we track one of the newer types?

@ryankurte
Copy link
Contributor Author

I'd be happy either way, just haven't yet had a moment to revisit / rebase to use the .into() approach, whatever y'all prefer?

@ryankurte
Copy link
Contributor Author

Went with the sealed approach which is a bit more complex but... also kindof nice in that you can click the trait and see the types it's implemented over more directly in the docs than looking via .into().

@twitchyliquid64
Copy link
Contributor

Thanks! Looks good to merge once we get CI green:

error[E0432]: unresolved import `cortex_m_0_6::peripheral::itm`
  --> /home/runner/.cargo/registry/src/github.1485827954.workers.dev-1ecc6299db9ec823/cortex-m-0.5.10/src/peripheral/mod.rs:91:46
   |
91 | pub use cortex_m_0_6::peripheral::{cbp, fpb, itm, tpiu};
   |                                              ^^^ no `itm` in `peripheral`

@twitchyliquid64
Copy link
Contributor

twitchyliquid64 commented Jan 25, 2021

Hmmm i may have to fix this on master

lmao wut
Rerunning CI, lets see if it was transient

@twitchyliquid64
Copy link
Contributor

twitchyliquid64 commented Jan 25, 2021

According to the embedded rust chat something mad derped in cortex-m, that might have been the issue

Fixed in rust-embedded/cortex-m#317

@twitchyliquid64 twitchyliquid64 merged commit e44aafd into atsamd-rs:master Jan 25, 2021
@ryankurte ryankurte deleted the usb-pad-trait branch January 25, 2021 03:55
@ryankurte ryankurte restored the usb-pad-trait branch June 20, 2021 04:48
kaizensparc pushed a commit to kaizensparc/atsamd that referenced this pull request Dec 24, 2021
* Introduce pad trait for USB GPIOs, backwards-compatible with v1

* Sealed USB traits

Co-authored-by: Twitch <[email protected]>
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