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

refactor stmf32f4xx chip code into multiple crates #1803

Merged
merged 3 commits into from May 4, 2020
Merged

refactor stmf32f4xx chip code into multiple crates #1803

merged 3 commits into from May 4, 2020

Conversation

hudson-ayers
Copy link
Contributor

@hudson-ayers hudson-ayers commented Apr 27, 2020

Pull Request Overview

This pull request refactors the stm32f4xx code into three crates. In addition to the base crate, it adds two new crates -- stm32f446re and stm32429zi -- which contain all code specific to those chip versions. So far, that is just a few NVIC IDs and the IRQS array.

Importantly, this PR removes any need for cargo features (#[cfg( feature = x)] statements) for functionality which differs between these two chips. It also absolves any need for cfg_if!() or other similar approach to enable clippy to pass on these crates, so I am closing #1794 in favor of this.

Testing Strategy

This pull request was tested by travis-ci and clippy.
I do not have any nucleo boards, so I can't test this on hardware.

TODO or Help Wanted

My approach to separating out the NVIC ids was to leave the shared IDs in stm32f4xx/src/nvic.rs and put chip-specific IDs in stm32f446re/src/stm32f446re_nvic.rs and stm32f429zi/src/stm32f429zi_nvic.rs. We don't currently use any of the chip-specific IDs, so the usability of this approach is pretty untested -- I think we would have to turn to something similar to the interrupt_service trait used by the nrf52 family of chips.

Documentation Updated

  • I updated all doc tests that used stm32f4xx, and didn't see any other documentation that needs updating, but could have missed something.

Formatting

  • Ran make formatall.

Cargo.toml Show resolved Hide resolved
chips/stm32f4xx/src/nvic.rs Show resolved Hide resolved
boards/nucleo_f429zi/src/main.rs Outdated Show resolved Hide resolved
gendx
gendx previously approved these changes Apr 28, 2020
bradjc
bradjc previously approved these changes May 1, 2020
ppannuto
ppannuto previously approved these changes May 1, 2020
Copy link
Member

@ppannuto ppannuto left a comment

Choose a reason for hiding this comment

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

bors d+

I don't have any STM hardware to test with, but this looks reasonable to me

@bors
Copy link
Contributor

bors bot commented May 1, 2020

✌️ hudson-ayers can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@gendx
Copy link
Contributor

gendx commented May 4, 2020

Can you remove the features from chips/stm32f4xx/Cargo.toml as well?

@hudson-ayers hudson-ayers dismissed stale reviews from ppannuto, bradjc, and gendx via 818832f May 4, 2020 12:44
@bors
Copy link
Contributor

bors bot commented May 4, 2020

✌️ hudson-ayers can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

Copy link
Member

@ppannuto ppannuto left a comment

Choose a reason for hiding this comment

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

bors d+

Same as before

@hudson-ayers
Copy link
Contributor Author

bors r+

Given that no one was able to do 1.5 release testing for the nucleo boards, I don't think it makes sense to wait for someone with hardware to test on this board.

@bors
Copy link
Contributor

bors bot commented May 4, 2020

Build succeeded:

@bors bors bot merged commit 8cdfd00 into master May 4, 2020
@bors bors bot deleted the stm-split branch May 4, 2020 13:14
bors bot added a commit that referenced this pull request May 6, 2020
1800: Remaining clippy allows/fixes r=hudson-ayers a=hudson-ayers

### Pull Request Overview

This pull request includes just the fixes from #1773, except those included in ~~#1794~~ #1803 or #1792 or #1796. It does not add clippy to the CI pipeline so that change can be reviewed seperately from these fixes. Once this PR and #1803 are merged, `tools/run_clippy.sh` will pass.

It includes several clippy allows, with justifications included, as well as fixes requested by clippy. I had to apply the clippy fixes manually as `cargo clippy --fix` does not seem to work with our current nightly version.


### Testing Strategy

This pull request was tested by Travis-CI and `run_clippy.sh`.


### TODO or Help Wanted

N/A

### Documentation Updated

- [ ] No updates are required.

### Formatting

- [x] Ran `make formatall`.


Co-authored-by: Hudson Ayers <hayers@stanford.edu>
Co-authored-by: Pat Pannuto <pat.pannuto@gmail.com>
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.

None yet

4 participants