Skip to content

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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Simplify Button interrupt logic. #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions examples/button_int.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ extern crate panic_itm;
use cortex_m_rt::entry;

use stm32f3_discovery::stm32f3xx_hal::interrupt;
use stm32f3_discovery::stm32f3xx_hal::prelude::*;
use stm32f3_discovery::stm32f3xx_hal::pac;
use stm32f3_discovery::stm32f3xx_hal::prelude::*;
use stm32f3_discovery::wait_for_interrupt;

use core::sync::atomic::{AtomicBool, Ordering};
Expand All @@ -31,6 +31,7 @@ fn EXTI0() {
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);
Copy link
Owner

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

Copy link
Author

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.


// initialize user leds
let mut gpioe = device_periphs.GPIOE.split(&mut reset_and_clock_control.ahb);
Expand All @@ -48,11 +49,7 @@ fn main() -> ! {
);
let mut status_led = leds.ld3;

button::interrupt::enable(
&device_periphs.EXTI,
&device_periphs.SYSCFG,
TriggerMode::Rising,
);
button::interrupt::enable(&device_periphs.EXTI, &syscfg, TriggerMode::Rising);

loop {
// check to see if flag was active and clear it
Expand Down
14 changes: 8 additions & 6 deletions src/button/interrupt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,10 @@ pub enum TriggerMode {
///
/// ```
/// let device_periphs = pac::Peripherals::take().unwrap();
/// button::interrupt::enable(&device_periphs.EXTI, &device_periphs.SYSCFG, TriggerMode::Rising);
/// let mut reset_and_clock_control = device_periphs.RCC.constrain();
/// let syscfg = device_periphs.SYSCFG.constrain(&mut reset_and_clock_control.apb2)
///
/// button::interrupt::enable(&device_periphs.EXTI, &syscfg, TriggerMode::Rising);
/// ```
pub fn enable(external_interrupts: &EXTI, sysconfig: &SYSCFG, mode: TriggerMode) {
// See chapter 14 of the reference manual
Expand All @@ -59,20 +62,19 @@ pub fn enable(external_interrupts: &EXTI, sysconfig: &SYSCFG, mode: TriggerMode)
}

fn configure_exti0(interrupt_mask: &IMR1) {
interrupt_mask.modify(|_, w| w.mr0().set_bit())
interrupt_mask.modify(|_, w| w.mr0().unmasked())
Copy link
Author

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.

Copy link
Owner

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.

Copy link
Owner

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());
Copy link
Owner

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 configure_rising_edge_trigger(rising_trigger_select: &RTSR1) {
rising_trigger_select.modify(|_, w| w.tr0().set_bit())
rising_trigger_select.modify(|_, w| w.tr0().enabled())
}

fn configure_falling_edge_trigger(falling_trigger_select: &FTSR1) {
falling_trigger_select.modify(|_, w| w.tr0().set_bit())
falling_trigger_select.modify(|_, w| w.tr0().enabled())
}

fn enable_exti0() {
Expand Down