-
Notifications
You must be signed in to change notification settings - Fork 21
Simplify Button interrupt logic. #47
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
base: master
Are you sure you want to change the base?
Conversation
1. SYSCFG needs to be constrained. The reason is that constrain looks like ``` fn constrain(self, apb2: &mut APB2) -> SysCfg { apb2.enr().modify(|_, w| w.syscfgen().enabled()); ///.... ``` which enables the SYSCFG. Without it, SYSCFG.exticr1 changes will not take effect (everything will still be 0s), which is not noticeable for the user button because it just happesn that PA0 config is the default, however for generic code consistency it seems we want to set a usable example 2. Try to use the seemingly clearer/safer alternatives when setting IMR1/RTSR1/FTSR1/EXTICR1: - use `enable` and `unmasked` instead of `set_bit` - use `pa0()` instead of a custom value write, which also removes the need for an unsafe block.
|
||
fn configure_exti0(interrupt_mask: &IMR1) { | ||
interrupt_mask.modify(|_, w| w.mr0().set_bit()) | ||
interrupt_mask.modify(|_, w| w.mr0().unmasked()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking out loud: are configure_exti0
and map_exti0_to_pa0
some typical Rust pattern? They are one-liners replaced with another one-liner, so I am wondering if it would be better for code readability to not have them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They’re not a Rust pattern. I was just naming things so I could understand the code without looking at the controller’s datasheet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ll be honest, I’ll need to look up the difference between set_bit
and unmasked
/enabled
fn map_exti0_to_pa0(external_interrupt_config: &EXTICR1) { | ||
const PORT_A_CONFIG: u8 = 0x000; | ||
external_interrupt_config.modify(|_, w| unsafe { w.exti0().bits(PORT_A_CONFIG) }); | ||
external_interrupt_config.modify(|_, w| w.exti0().pa0()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Heh. I wonder when the HAL added this.
When I first wrote this, I had to use the direct write in order to avoid having a memory safe handle to the PORTA config register.
I want to look at this more closely.
fn main() -> ! { | ||
let device_periphs = pac::Peripherals::take().unwrap(); | ||
let mut reset_and_clock_control = device_periphs.RCC.constrain(); | ||
let syscfg = device_periphs.SYSCFG.constrain(&mut reset_and_clock_control.apb2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: look up the APB2 register
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not familiar with it, just figured from the docs that I need SYSCFG configured, so I passed in whatever the HAL asked me to pass in.
I was looking to learn how to use my STM32F3Discovery board by looking over the stm32f3-discovery implementation and attempted to implement my own button interrupt (on PB1 using EXTI1). I encountered some difficulty because:
I am learning this stuff, so feedback would be highly appreciated. Thank you!
Actual changes from the log:
like
which enables the SYSCFG. Without it, SYSCFG.exticr1 changes will not
take effect (everything will still be 0s), which is not noticeable for
the user button because it just happesn that PA0 config is the default,
however for generic code consistency it seems we want to set a usable
example
IMR1/RTSR1/FTSR1/EXTICR1:
enable
andunmasked
instead ofset_bit
pa0()
instead of a custom value write, which also removesthe need for an unsafe block.