Skip to content

Commit e5a8b96

Browse files
committed
Refactor display to take DisplayPins
This change makes use of the approach from @mattheww in mattheww/rmicrobit where the display method (`initialise_display` and `handle_display_event`) take a `DisplayPins` struct rather than a reference to the GPIO peripheral. This struct is never used but ensures nothing else is accidentally using the pins it uses. It then modifies the pins directly on the peripheral so that multiple can be updated together. I have moved and renamed the `led::Pins` struct as it seems to make more sense in the `gpio` module as it's used in `led` and `display` now. Finally, I have introduced a `display_pins` macro, mostly because I was getting tired of copying around the initialization of the big pin struct.
1 parent ef72e44 commit e5a8b96

File tree

7 files changed

+107
-79
lines changed

7 files changed

+107
-79
lines changed

examples/led_blocking.rs

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,9 @@ use panic_halt as _;
66

77
use cortex_m_rt::entry;
88

9-
use microbit::hal::{
10-
gpio::{p0::Parts as P0Parts, Level},
11-
prelude::*,
12-
Timer,
9+
use microbit::{
10+
display_pins,
11+
hal::{gpio::p0::Parts as P0Parts, prelude::*, Timer},
1312
};
1413

1514
use microbit::led;
@@ -21,20 +20,7 @@ fn main() -> ! {
2120
let p0parts = P0Parts::new(p.GPIO);
2221

2322
// Display
24-
let pins = led::Pins {
25-
row1: p0parts.p0_13.into_push_pull_output(Level::Low),
26-
row2: p0parts.p0_14.into_push_pull_output(Level::Low),
27-
row3: p0parts.p0_15.into_push_pull_output(Level::Low),
28-
col1: p0parts.p0_04.into_push_pull_output(Level::Low),
29-
col2: p0parts.p0_05.into_push_pull_output(Level::Low),
30-
col3: p0parts.p0_06.into_push_pull_output(Level::Low),
31-
col4: p0parts.p0_07.into_push_pull_output(Level::Low),
32-
col5: p0parts.p0_08.into_push_pull_output(Level::Low),
33-
col6: p0parts.p0_09.into_push_pull_output(Level::Low),
34-
col7: p0parts.p0_10.into_push_pull_output(Level::Low),
35-
col8: p0parts.p0_11.into_push_pull_output(Level::Low),
36-
col9: p0parts.p0_12.into_push_pull_output(Level::Low),
37-
};
23+
let pins = display_pins!(p0parts);
3824
let mut leds = led::Display::new(pins);
3925

4026
#[allow(non_snake_case)]

examples/led_nonblocking.rs

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,13 @@ use cortex_m_rt::entry;
1111

1212
use microbit::{
1313
display::{self, image::GreyscaleImage, Display, Frame, MicrobitDisplayTimer, MicrobitFrame},
14-
hal::rtc::{Rtc, RtcInterrupt},
15-
pac::{self, interrupt, GPIO, RTC0, TIMER1},
14+
display_pins,
15+
gpio::DisplayPins,
16+
hal::{
17+
gpio::p0::Parts as P0Parts,
18+
rtc::{Rtc, RtcInterrupt},
19+
},
20+
pac::{self, interrupt, RTC0, TIMER1},
1621
};
1722

1823
fn heart_image(inner_brightness: u8) -> GreyscaleImage {
@@ -29,7 +34,7 @@ fn heart_image(inner_brightness: u8) -> GreyscaleImage {
2934
// We use TIMER1 to drive the display, and RTC0 to update the animation.
3035
// We set the TIMER1 interrupt to a higher priority than RTC0.
3136

32-
static GPIO: Mutex<RefCell<Option<GPIO>>> = Mutex::new(RefCell::new(None));
37+
static LED_PINS: Mutex<RefCell<Option<DisplayPins>>> = Mutex::new(RefCell::new(None));
3338
static ANIM_TIMER: Mutex<RefCell<Option<Rtc<RTC0>>>> = Mutex::new(RefCell::new(None));
3439
static DISPLAY_TIMER: Mutex<RefCell<Option<MicrobitDisplayTimer<TIMER1>>>> =
3540
Mutex::new(RefCell::new(None));
@@ -52,9 +57,11 @@ fn main() -> ! {
5257
rtc0.enable_counter();
5358

5459
let mut timer = MicrobitDisplayTimer::new(p.TIMER1);
55-
let mut gpio = p.GPIO;
56-
display::initialise_display(&mut timer, &mut gpio);
57-
*GPIO.borrow(cs).borrow_mut() = Some(gpio);
60+
61+
let p0parts = P0Parts::new(p.GPIO);
62+
let mut pins = display_pins!(p0parts);
63+
display::initialise_display(&mut timer, &mut pins);
64+
*LED_PINS.borrow(cs).borrow_mut() = Some(pins);
5865
*ANIM_TIMER.borrow(cs).borrow_mut() = Some(rtc0);
5966
*DISPLAY_TIMER.borrow(cs).borrow_mut() = Some(timer);
6067
*DISPLAY.borrow(cs).borrow_mut() = Some(Display::new());
@@ -78,9 +85,9 @@ fn main() -> ! {
7885
fn TIMER1() {
7986
cortex_m::interrupt::free(|cs| {
8087
if let Some(timer) = DISPLAY_TIMER.borrow(cs).borrow_mut().as_mut() {
81-
if let Some(gpio) = GPIO.borrow(cs).borrow_mut().as_mut() {
88+
if let Some(pins) = LED_PINS.borrow(cs).borrow_mut().as_mut() {
8289
if let Some(d) = DISPLAY.borrow(cs).borrow_mut().as_mut() {
83-
display::handle_display_event(d, timer, gpio);
90+
display::handle_display_event(d, timer, pins);
8491
}
8592
}
8693
}

examples/led_rtfm.rs

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ use panic_halt as _;
1111

1212
use microbit::{
1313
display::{self, image::GreyscaleImage, Display, Frame, MicrobitDisplayTimer, MicrobitFrame},
14-
hal::rtc::{Rtc, RtcInterrupt},
14+
display_pins,
15+
gpio::DisplayPins,
16+
hal::{
17+
gpio::p0::Parts as P0Parts,
18+
rtc::{Rtc, RtcInterrupt},
19+
},
1520
pac,
1621
};
1722
use rtic::app;
@@ -30,15 +35,15 @@ fn heart_image(inner_brightness: u8) -> GreyscaleImage {
3035
#[app(device = microbit::pac, peripherals = true)]
3136
const APP: () = {
3237
struct Resources {
33-
gpio: pac::GPIO,
38+
display_pins: DisplayPins,
3439
display_timer: MicrobitDisplayTimer<pac::TIMER1>,
3540
anim_timer: Rtc<pac::RTC0>,
3641
display: Display<MicrobitFrame>,
3742
}
3843

3944
#[init]
4045
fn init(cx: init::Context) -> init::LateResources {
41-
let mut p: pac::Peripherals = cx.device;
46+
let p: pac::Peripherals = cx.device;
4247

4348
// Starting the low-frequency clock (needed for RTC to work)
4449
p.CLOCK.tasks_lfclkstart.write(|w| unsafe { w.bits(1) });
@@ -53,23 +58,27 @@ const APP: () = {
5358
rtc0.enable_counter();
5459

5560
let mut timer = MicrobitDisplayTimer::new(p.TIMER1);
56-
display::initialise_display(&mut timer, &mut p.GPIO);
61+
62+
let p0parts = P0Parts::new(p.GPIO);
63+
let mut pins = display_pins!(p0parts);
64+
65+
display::initialise_display(&mut timer, &mut pins);
5766

5867
init::LateResources {
59-
gpio: p.GPIO,
68+
display_pins: pins,
6069
display_timer: timer,
6170
anim_timer: rtc0,
6271
display: Display::new(),
6372
}
6473
}
6574

6675
#[task(binds = TIMER1, priority = 2,
67-
resources = [display_timer, gpio, display])]
76+
resources = [display_timer, display_pins, display])]
6877
fn timer1(mut cx: timer1::Context) {
6978
display::handle_display_event(
7079
&mut cx.resources.display,
7180
cx.resources.display_timer,
72-
cx.resources.gpio,
81+
cx.resources.display_pins,
7382
);
7483
}
7584

src/display/control.rs

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const ROW_BITS: u32 = pin_bits(&ROWS);
3030
/// This implements the `DisplayControl` trait.
3131
///
3232
/// [`DisplayControl`]: tiny_led_matrix::DisplayControl
33-
pub(crate) struct MicrobitGpio<'a>(pub &'a pac::GPIO);
33+
pub(crate) struct MicrobitGpio;
3434

3535
/// Returns the GPIO pin numbers corresponding to the columns in a Columnt et.
3636
fn column_pins(mut cols: u32) -> u32 {
@@ -50,38 +50,42 @@ fn column_pins(mut cols: u32) -> u32 {
5050
/// state it would have after system reset.
5151
///
5252
/// [`DisplayControl`]: tiny_led_matrix::DisplayControl
53-
impl DisplayControl for MicrobitGpio<'_> {
53+
impl DisplayControl for MicrobitGpio {
5454
fn initialise_for_display(&mut self) {
55-
let gpio = &self.0;
56-
for ii in COLS.iter() {
57-
gpio.pin_cnf[*ii].write(|w| w.dir().output());
58-
}
59-
for ii in ROWS.iter() {
60-
gpio.pin_cnf[*ii].write(|w| w.dir().output());
61-
}
55+
unsafe {
56+
let gpio = &*pac::GPIO::ptr();
57+
for ii in COLS.iter() {
58+
gpio.pin_cnf[*ii].write(|w| w.dir().output());
59+
}
60+
for ii in ROWS.iter() {
61+
gpio.pin_cnf[*ii].write(|w| w.dir().output());
62+
}
6263

63-
// Set all cols high.
64-
gpio.outset
65-
.write(|w| unsafe { w.bits(COLS.iter().map(|pin| 1 << pin).sum()) });
64+
// Set all cols high.
65+
gpio.outset
66+
.write(|w| w.bits(COLS.iter().map(|pin| 1 << pin).sum()));
67+
}
6668
}
6769

6870
fn display_row_leds(&mut self, row: usize, cols: u32) {
69-
let gpio = &self.0;
70-
// To light an LED, we set the row bit and clear the col bit.
71-
let rows_to_set = 1 << ROWS[row];
72-
let rows_to_clear = ROW_BITS ^ rows_to_set;
71+
unsafe {
72+
let gpio = &*pac::GPIO::ptr();
73+
// To light an LED, we set the row bit and clear the col bit.
74+
let rows_to_set = 1 << ROWS[row];
75+
let rows_to_clear = ROW_BITS ^ rows_to_set;
7376

74-
let cols_to_clear = column_pins(cols);
75-
let cols_to_set = COL_BITS ^ cols_to_clear;
77+
let cols_to_clear = column_pins(cols);
78+
let cols_to_set = COL_BITS ^ cols_to_clear;
7679

77-
gpio.outset
78-
.write(|w| unsafe { w.bits(rows_to_set | cols_to_set) });
79-
gpio.outclr
80-
.write(|w| unsafe { w.bits(rows_to_clear | cols_to_clear) });
80+
gpio.outset.write(|w| w.bits(rows_to_set | cols_to_set));
81+
gpio.outclr.write(|w| w.bits(rows_to_clear | cols_to_clear));
82+
}
8183
}
8284

8385
fn light_current_row_leds(&mut self, cols: u32) {
84-
let gpio = &self.0;
85-
gpio.outclr.write(|w| unsafe { w.bits(column_pins(cols)) });
86+
unsafe {
87+
let gpio = &*crate::pac::GPIO::ptr();
88+
gpio.outclr.write(|w| w.bits(column_pins(cols)));
89+
}
8690
}
8791
}

src/display/mod.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ pub mod image;
134134
pub use matrix::MicrobitFrame;
135135
pub use timer::MicrobitDisplayTimer;
136136

137-
use crate::hal::timer::Instance;
137+
use crate::{gpio::DisplayPins, hal::timer::Instance};
138138

139139
use control::MicrobitGpio;
140140

@@ -151,9 +151,9 @@ use control::MicrobitGpio;
151151
/// ```
152152
pub fn initialise_display<T: Instance>(
153153
timer: &mut MicrobitDisplayTimer<T>,
154-
gpio: &mut crate::pac::GPIO,
154+
_pins: &mut DisplayPins,
155155
) {
156-
tiny_led_matrix::initialise_control(&mut MicrobitGpio(gpio));
156+
tiny_led_matrix::initialise_control(&mut MicrobitGpio {});
157157
tiny_led_matrix::initialise_timer(timer);
158158
}
159159

@@ -186,7 +186,7 @@ pub fn initialise_display<T: Instance>(
186186
pub fn handle_display_event<T: Instance>(
187187
display: &mut Display<MicrobitFrame>,
188188
timer: &mut MicrobitDisplayTimer<T>,
189-
gpio: &mut crate::pac::GPIO,
189+
_pins: &mut DisplayPins,
190190
) {
191-
display.handle_event(timer, &mut MicrobitGpio(gpio));
191+
display.handle_event(timer, &mut MicrobitGpio {});
192192
}

src/gpio.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,43 @@ pub type ROW1 = p0::P0_13<Output<PushPull>>;
2727
pub type ROW2 = p0::P0_14<Output<PushPull>>;
2828
pub type ROW3 = p0::P0_15<Output<PushPull>>;
2929

30+
pub struct DisplayPins {
31+
pub col1: COL1,
32+
pub col2: COL2,
33+
pub col3: COL3,
34+
pub col4: COL4,
35+
pub col5: COL5,
36+
pub col6: COL6,
37+
pub col7: COL7,
38+
pub col8: COL8,
39+
pub col9: COL9,
40+
pub row1: ROW1,
41+
pub row2: ROW2,
42+
pub row3: ROW3,
43+
}
44+
45+
#[macro_export]
46+
macro_rules! display_pins {
47+
( $p0parts:expr ) => {{
48+
use microbit::{gpio::DisplayPins, hal::gpio::Level};
49+
50+
DisplayPins {
51+
row1: $p0parts.p0_13.into_push_pull_output(Level::Low),
52+
row2: $p0parts.p0_14.into_push_pull_output(Level::Low),
53+
row3: $p0parts.p0_15.into_push_pull_output(Level::Low),
54+
col1: $p0parts.p0_04.into_push_pull_output(Level::Low),
55+
col2: $p0parts.p0_05.into_push_pull_output(Level::Low),
56+
col3: $p0parts.p0_06.into_push_pull_output(Level::Low),
57+
col4: $p0parts.p0_07.into_push_pull_output(Level::Low),
58+
col5: $p0parts.p0_08.into_push_pull_output(Level::Low),
59+
col6: $p0parts.p0_09.into_push_pull_output(Level::Low),
60+
col7: $p0parts.p0_10.into_push_pull_output(Level::Low),
61+
col8: $p0parts.p0_11.into_push_pull_output(Level::Low),
62+
col9: $p0parts.p0_12.into_push_pull_output(Level::Low),
63+
}
64+
}};
65+
}
66+
3067
/* buttons */
3168
pub type BTN_A = p0::P0_17<Input<Floating>>;
3269
pub type BTN_B = p0::P0_26<Input<Floating>>;

src/led.rs

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::hal::{
55
prelude::*,
66
};
77

8-
use crate::gpio::{COL1, COL2, COL3, COL4, COL5, COL6, COL7, COL8, COL9, ROW1, ROW2, ROW3};
8+
use crate::gpio::DisplayPins;
99

1010
use embedded_hal::blocking::delay::DelayUs;
1111

@@ -21,21 +21,6 @@ const LED_LAYOUT: [[(usize, usize); 5]; 5] = [
2121
[(2, 2), (1, 6), (2, 0), (1, 5), (2, 1)],
2222
];
2323

24-
pub struct Pins {
25-
pub col1: COL1,
26-
pub col2: COL2,
27-
pub col3: COL3,
28-
pub col4: COL4,
29-
pub col5: COL5,
30-
pub col6: COL6,
31-
pub col7: COL7,
32-
pub col8: COL8,
33-
pub col9: COL9,
34-
pub row1: ROW1,
35-
pub row2: ROW2,
36-
pub row3: ROW3,
37-
}
38-
3924
/// Array of all the LEDs in the 5x5 display on the board
4025
pub struct Display {
4126
delay_ms: u32,
@@ -46,7 +31,7 @@ pub struct Display {
4631
impl Display {
4732
/// Initializes all the user LEDs
4833
#[allow(clippy::too_many_arguments)]
49-
pub fn new(pins: Pins) -> Self {
34+
pub fn new(pins: DisplayPins) -> Self {
5035
let mut retval = Display {
5136
delay_ms: DEFAULT_DELAY_MS,
5237
rows: [

0 commit comments

Comments
 (0)