Skip to content

Rework Interrupt enum for MSP430 #460

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 9, 2020
Merged

Rework Interrupt enum for MSP430 #460

merged 1 commit into from
Aug 9, 2020

Conversation

therealprof
Copy link
Contributor

This also removes the bare-metal dependency from PACs created for
MSP430, as requested in
#455 (comment)

Signed-off-by: Daniel Egger [email protected]

@therealprof therealprof requested a review from a team as a code owner August 5, 2020 20:37
@rust-highfive
Copy link

r? @burrbull

(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-tools labels Aug 5, 2020
@therealprof
Copy link
Contributor Author

CC @cr1901

@cr1901
Copy link
Contributor

cr1901 commented Aug 6, 2020

@therealprof Unable to test right now... it looks fine to me. Since msp430 crate at present relies on bare_metal to import CriticalSection and Mutex, I think I'll need to bump the patch version and require that anyone generating a PAC use at least a (to-be-published) v0.2.3 version of msp430 crate.

However, this doesn't affect the public API and it's an impl detail that msp430 still depends on bare_metal, so no rush.

@therealprof
Copy link
Contributor Author

Since the generated PACs don't depend on bare-metal anymore you are free to use any version you'd like in your application or other dependencies without conflict.

@therealprof
Copy link
Contributor Author

r? @adamgreig

@rust-highfive rust-highfive assigned adamgreig and unassigned burrbull Aug 6, 2020
adamgreig
adamgreig reviewed Aug 6, 2020
View reviewed changes
src/generate/interrupt.rs Outdated
@@ -176,7 +189,7 @@ pub fn render(
}
};

if target == Target::CortexM || target == Target::Msp430 {
if target == Target::CortexM {
root.extend(interrupt_enum);
} else {
Copy link
Member

Choose a reason for hiding this comment

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

Won't this else branch still trigger, causing #interrupt_enum to end up in mod_items along with the try_from impl etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I wonder why this didn't cause any CI problems...

@therealprof
Rework Interrupt enum for MSP430
1512a6f 
This also removes the `bare-metal` dependency from PACs created for
MSP430, as requested in
#455 (comment)

Signed-off-by: Daniel Egger <[email protected]>
@therealprof
Copy link
Contributor Author

@adamgreig Are we good to go now?

adamgreig
adamgreig approved these changes Aug 9, 2020
View reviewed changes
Copy link
Member

@adamgreig adamgreig left a comment

Choose a reason for hiding this comment

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

bors merge

@bors
Copy link
Contributor

bors bot commented Aug 9, 2020

Build succeeded:

@bors bors bot merged commit b064ab0 into master Aug 9, 2020
@bors bors bot deleted the MSP430-Interrupt branch August 9, 2020 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adamgreig adamgreig adamgreig approved these changes

Assignees

@adamgreig adamgreig

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@therealprof @rust-highfive @cr1901 @adamgreig @burrbull