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

Add new InterruptNumber trait #241

Merged
merged 2 commits into from
Jul 22, 2020
Merged

Add new InterruptNumber trait #241

merged 2 commits into from
Jul 22, 2020

Conversation

adamgreig
Copy link
Member

This is a first go at the new trait needed for rust-embedded/svd2rust#455 since we removed Nr from bare-metal.

In this case I've written it as unsafe trait InterruptNumber: Into<u16> rather than providing a conversion method inside the trait; I think this is neat and idiomatic but please correct me if there's a reason to not do it like this.

Here's a playground link showing an example implementation.

@rust-highfive
Copy link

r? @korken89

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m labels Jul 9, 2020
@therealprof
Copy link
Contributor

I like it! I'm also a big fan of the Into<u16> idea. 😉

jonas-schievink
jonas-schievink previously approved these changes Jul 9, 2020
Copy link
Contributor

@jonas-schievink jonas-schievink left a comment

Choose a reason for hiding this comment

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

LGTM. Haven't seen the Into<X> supertrait thing anywhere before, but I don't mind it.

@adamgreig
Copy link
Member Author

I've updated to fix the failing CI (due to the use of some &InterruptNumber types; Into isn't implemented on reference types). The new trait now requires Copy and I've removed use of references.

@therealprof therealprof added the nominated Issue nominated as discussion topic for the Embedded WG meeting label Jul 20, 2020
@therealprof
Copy link
Contributor

As discussed in todays meeting this should be adjusted to lose the Into<u16> trick and provide a number method instead. Everything else seems ready to go.

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

Missing CHANGELOG entry. ;)

Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

Thank you! Let's land this then.

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 22, 2020

Build succeeded:

@bors bors bot merged commit 9dee813 into master Jul 22, 2020
@therealprof therealprof removed the nominated Issue nominated as discussion topic for the Embedded WG meeting label Jul 28, 2020
@jonas-schievink jonas-schievink deleted the interrupt-number branch January 25, 2021 23:40
adamgreig pushed a commit that referenced this pull request Jan 12, 2022
241: Fix unused doc lint firing on `#[pre_init]` and add a test r=adamgreig a=jonas-schievink



Co-authored-by: Jonas Schievink <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-cortex-m
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants