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

treewide: spi: Enforce proper CS pin usage #103

Merged
merged 3 commits into from
Nov 18, 2020
Merged

Conversation

jonahd-g
Copy link
Contributor

/resolves #27

Makes the proposed change to the SPI macro and the usage by ATmega328p. Waiting for review and approval before making an attempt to update all other usages.

Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

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

Nice, thank you very much! The design is solid, I've left a few comments but apart from that you can go ahead and update all the other HAL crates and examples :)

avr-hal-generic/src/spi.rs Show resolved Hide resolved
avr-hal-generic/src/spi.rs Outdated Show resolved Hide resolved
avr-hal-generic/src/spi.rs Show resolved Hide resolved
avr-hal-generic/src/spi.rs Outdated Show resolved Hide resolved
@Rahix Rahix added bug Something isn't working hal-api API design for the different components of avr-hal labels Nov 17, 2020
@jonahd-g
Copy link
Contributor Author

The design is solid

Lol you don't get to say that when I mostly copied from your comment 😉

@Rahix
Copy link
Owner

Rahix commented Nov 17, 2020

Lol you don't get to say that when I mostly copied from your comment 😉

Haha, I did not at all remember what I wrote back then, please forgive me :D

@jonahd-g jonahd-g marked this pull request as ready for review November 17, 2020 22:26
@jonahd-g jonahd-g requested a review from Rahix November 17, 2020 22:27
Copy link
Owner

@Rahix Rahix left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a lot!

@@ -93,14 +93,13 @@ pub fn delay_us(us: u16) {
///
/// let mut pins = arduino_leonardo::Pins::new(dp.PORTB, dp.PORTC, dp.PORTD, dp.PORTE);
///
/// pins.led_rx.into_output(&mut pins.ddr); // Chip Select must be set to output mode.
///
/// // Create SPI interface.
/// let mut spi = arduino_leonardo::spi::Spi::new(
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
/// let mut spi = arduino_leonardo::spi::Spi::new(
/// let (mut spi, mut cs) = arduino_leonardo::spi::Spi::new(

@Rahix Rahix changed the title Added CS pin and wrapper struct to SPI macro, updated usage in ATmega328p treewide: spi: Enforce proper CS pin usage Nov 18, 2020
@Rahix Rahix merged commit fc0303b into Rahix:master Nov 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working hal-api API design for the different components of avr-hal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SPI: CS must be configured before SPI peripheral
2 participants