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

ExtiPin trait for GPIOs #125

Merged
merged 12 commits into from
Dec 5, 2019
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](http://keepachangelog.com/)
and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Added

- Added support for `ExtiPin` pin traits

## [v0.5.0] - 2019-12-03

### Added
Expand Down
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ features = ["stm32f103", "rt", "stm32-usbd"]
[[example]]
name = "timer-interrupt-rtfm"
required-features = ["rt"]
[[example]]
name = "exti"
required-features = ["rt"]

[dependencies]
cortex-m = "0.6.0"
Expand Down
68 changes: 68 additions & 0 deletions examples/exti.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
//! Turns the user LED on
//!
//! Listens for interrupts on the pa7 pin. On any rising or falling edge, toggles
//! the pc13 pin (which is connected to the LED on the blue pill board, hence the `led` name).

#![no_main]
#![no_std]

use panic_halt as _;

use stm32f1xx_hal::{
prelude::*,
pac
};
use cortex_m_rt::entry;
use pac::interrupt;
use core::mem::MaybeUninit;
use embedded_hal::digital::v2::OutputPin;
use stm32f1xx_hal::gpio::*;

// These two are owned by the ISR. main() may only access them during the initialization phase,
// where the interrupt is not yet enabled (i.e. no concurrent accesses can occur).
// After enabling the interrupt, main() may not have any references to these objects any more.
// For the sake of minimalism, we do not use RTFM here, which would be the better way.
static mut LED : MaybeUninit<stm32f1xx_hal::gpio::gpioc::PC13<Output<PushPull>>> = MaybeUninit::uninit();
static mut EXTI : MaybeUninit<stm32f1xx_hal::gpio::gpioa::PA7<Input<Floating>>> = MaybeUninit::uninit();

#[interrupt]
fn EXTI9_5() {
let led = unsafe { &mut *LED.as_mut_ptr()};
let exti = unsafe { &mut *EXTI.as_mut_ptr()};

if exti.check_interrupt() {
led.toggle();

// if we don't clear this bit, the ISR would trigger indefinitely
exti.clear_interrupt_pending_bit();
}
}

#[entry]
fn main() -> ! {
// initialization phase
let p = pac::Peripherals::take().unwrap();
let cp = cortex_m::peripheral::Peripherals::take().unwrap();
{
// the scope ensures that the exti reference is dropped before the first ISR can be executed.
Copy link
Member

Choose a reason for hiding this comment

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

I'm assuming you do this to avoid a race condition when between main and the interrupt handler.

However, I don't think holding a reference to something can trigger a race condition, that can only happen if you actually use the reference.

Or is this more of a warning to users that they shouldn't use exti after nvic.enable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It' both. It's a warning, and also: Isn't it UB that two mutable references to the same data exist? (Regardless of them being used?)

Copy link
Member

Choose a reason for hiding this comment

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

I'm not entirely sure. My guess is that it's not for the same reason that working with pointers is safe until you actually dereference them, which requires unsafe but I may be wrong

Copy link
Contributor

Choose a reason for hiding this comment

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

With non lexical lifetime, it's safe if you don't use the mutable reference before enabling interrupt, as it would be accepted by the compiler to create another &mut after the enable line.

For examples, I prefer a static mut option to a static mut maybeuninit, less unsafe things to think about.


let mut rcc = p.RCC.constrain();
let mut gpioa = p.GPIOA.split(&mut rcc.apb2);
let mut gpioc = p.GPIOC.split(&mut rcc.apb2);
let mut afio = p.AFIO.constrain(&mut rcc.apb2);

let led = unsafe { &mut *LED.as_mut_ptr()};
*led = gpioc.pc13.into_push_pull_output(&mut gpioc.crh);
Copy link
Contributor

Choose a reason for hiding this comment

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

let led = gpioc.pc13.into_push_pull_output(&mut gpioc.crh);
unsafe { LED = MaybeUninit::new(led) };

Copy link
Contributor

Choose a reason for hiding this comment

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

no need of scope with this.


let exti = unsafe { &mut *EXTI.as_mut_ptr()};
*exti = gpioa.pa7.into_floating_input(&mut gpioa.crl);
exti.make_interrupt_source(&mut afio);
exti.trigger_on_edge(&p.EXTI, Edge::RISING_FALLING);
exti.enable_interrupt(&p.EXTI);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same pattern:

let exti = gpioa.pa7.into_floating_input(&mut gpioa.crl);
exti.make_interrupt_source(&mut afio);
exti.trigger_on_edge(&p.EXTI, Edge::RISING_FALLING);
exti.enable_interrupt(&p.EXTI);
unsafe { EXTI = MaybeUninit::new(exti) };

} // initialization ends here

let mut nvic = cp.NVIC;
nvic.enable(pac::Interrupt::EXTI9_5);

loop {}
}
Loading