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.

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

Add more peripherals for the stm32f030 #6

Merged
merged 20 commits into from
Dec 19, 2018
Merged

Add more peripherals for the stm32f030 #6

merged 20 commits into from
Dec 19, 2018

Conversation

david-sawatzke
Copy link
Member

@david-sawatzke david-sawatzke commented Dec 18, 2018 •
edited
Loading

TODO

  • (Maybe) Port examples so they work on all variants (Opened a separate issue for that)

HarkonenBade
HarkonenBade reviewed Dec 18, 2018
View reviewed changes
src/serial.rs

impl<USART> embedded_hal::serial::Read<u8> for Rx<USART>
where
USART: Deref<Target = SerialRegisterBlock>,
Copy link
Member

Choose a reason for hiding this comment

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

This again feels kinda sketchy to me, as it is relying on all the USART peripherals continuing to use the same register block definition.

Copy link
Member Author

Choose a reason for hiding this comment

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

If it's going to break, we're getting a compile error, so in that's ok. In the worst case you can do some macro based approach where you can separately implement the different register blocks.

@david-sawatzke
Copy link
Member Author

david-sawatzke commented Dec 18, 2018 •
edited
Loading

Added all possible pin combinations and split up the pin traits for Serial. Also wrapped pin traits in a macro, otherwise the selection based on features is really large

therealprof
therealprof reviewed Dec 18, 2018
View reviewed changes
src/serial.rs Outdated
usart_pins! {
// TODO WTF look at this again, in the datasheet PB10 is both tx and rx
// USART3: (gpiob::PB10, AF4, gpiob::PA11, AF4),
USART3: (gpioc::PC4<Alternate<AF1>>, gpioc::PC5<Alternate<AF1>>),
Copy link
Member

Choose a reason for hiding this comment

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

Instead of specifying pairs wouldn't it make more sense to actually single out the TX/RX functions and let the user chose any possible combination? Using this macro in this way means you're passing on a few possible combinations, e.g. you use also use PC10 and PC5 for USART3, or PC4 and PC11.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thats the whole purpose of the separate Rx and Tx traits, the combinations are done by the templates. The only reason i defined them pairwise is because that what ST seems to have done and it seemed cleaner this way.

Copy link
Member

Choose a reason for hiding this comment

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

If it would be helpful I could probably rig that macro so that you just specify each USART once and then list all the TX pins and then all the RX pins.

Copy link
Member

Choose a reason for hiding this comment

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

@HarkonenBade That sounds like a good idea.

Copy link
Member Author

Choose a reason for hiding this comment

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

@HarkonenBade Agree. We could also reuse that for the spi, where the pin combinations aren't that clear.

Copy link
Member

Choose a reason for hiding this comment

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

Done, if @david-sawatzke could pull the fresh commit from https://github.com/HarkonenBade/stm32f0xx-hal/tree/stm32f030_v2 that would be great.

Copy link
Member

Choose a reason for hiding this comment

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

I took some slight liberties with making the macro syntax a a bit more verbose, but I think it makes it a lot clearer what is going on.

Copy link
Member Author

Choose a reason for hiding this comment

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

@HarkonenBade Yeah, that's better. The syntax also seems more "rusty".

@david-sawatzke david-sawatzke mentioned this pull request Dec 19, 2018
Merged
@therealprof
Copy link
Member

Is this still WIP? Looks pretty much complete to me.

@david-sawatzke
Copy link
Member Author

I'd also like to port spi & i2c to the same approach as serial and add the 2nd instance

@david-sawatzke
Copy link
Member Author

david-sawatzke commented Dec 19, 2018 •
edited
Loading

Currently getting a rustc panic

thread 'main' panicked at 'byte index 1177 is out of bounds of `crate::stm32::$SPI`', libcore/str/mod.rs:2094:9
note: Run with `RUST_BACKTRACE=1` for a backtrace.

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

EDIT:
Probably this issue rust-lang/rust#56411

david-sawatzke and others added 19 commits December 19, 2018 13:16
@david-sawatzke
Add spi for stm32f030 (incomplete)
b724b52
@david-sawatzke
Add i2c for stm32f030 (incomplete)
66b869d
@david-sawatzke
Enable macro less implementations of multiple usarts
d059c11
@david-sawatzke
Use macros for initalization code
a26e44c
@david-sawatzke
Remove duplicate features
50f76b9
@david-sawatzke
Add some documentation to serial
1c21cc7
@david-sawatzke
Enable gpiod, since stm32f0 crate was updated
f3fe126
@david-sawatzke
Add serial single pin traits and implement all af pins
4d5f926
@david-sawatzke
Enable usart6, since stm32f0 crate was updated
b8ec4e4
@HarkonenBade @david-sawatzke
Make USART pins macro more flexible
e5f7a2c
@david-sawatzke
Adjust note for confusing pin
0e03a32
@david-sawatzke
Settle PB10
928fd64 
Found a way to confirm binding
@david-sawatzke
Implement single pin trait based AF for I2C
f1f2b32
@david-sawatzke
Implement single pin trait based AF for SPI
527f182
@david-sawatzke
Remove unecessary serial pin restriction
ad830af
@david-sawatzke
Implement multiple spi instances (BROKEN)
14aab2a
@david-sawatzke
Fix the rustc crash and other things
78b252a
@david-sawatzke
Add pins for SPI2
a51bb0e
@david-sawatzke
Generalize i2c and port to i2c2
2aa1d1b
@david-sawatzke david-sawatzke changed the title [WIP] Add more peripherals for the stm32f030 Add more peripherals for the stm32f030 Dec 19, 2018
@david-sawatzke
Copy link
Member Author

I think this is ready to merge now

therealprof
therealprof reviewed Dec 19, 2018
View reviewed changes
Cargo.toml
stm32f030x8 = ["stm32f030", "stm32f0/stm32f0x0"]
stm32f030xc = ["stm32f030", "stm32f0/stm32f0x0"]
stm32f030x8 = ["stm32f030"]
stm32f030xc = ["stm32f030"]
Copy link
Member

Choose a reason for hiding this comment

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

Snuck in another unrelated change, eh? ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

It's technically not completely unrelated

therealprof
therealprof approved these changes Dec 19, 2018
View reviewed changes
Copy link
Member

@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.

LGTM, thanks.

@therealprof therealprof merged commit a959593 into stm32-rs:master Dec 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HarkonenBade HarkonenBade HarkonenBade left review comments

@therealprof therealprof therealprof approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@david-sawatzke @therealprof @HarkonenBade