Merge #55

55: Refactor LED display modules r=therealprof a=robyoung

Addresses #52 

This change is pretty bold. I'm not sure if it's the right direction to go.

Rename and refactor the two LED display modules so that their APIs are closer to each other and hopefully more familiar to people coming from the micropython APIs.

*Blocking API*
```rust
let mut display = Display::new(pins);
loop {
  display.show(&mut timer, heart, 1000);
  display.clear();
  timer.delay_ms(250u32);
}
```

*Non-blocking API*
```rust
let mut display = Display::new(timer, pins);
display.initialise();

loop {
  display.show(heart);
  timer2.delay_ms(1000u32);
  display.clear();
  timer2.delay_ms(500u32);
}

// in a timer interrupt
display.handle_display_event();
```

## Rename the modules
Move the two modules under a common `display` as `blocking` and `nonblocking` and rename all led examples to display.

I went with display over led as this is the name used in the micropython API.

I went with `blocking` and `nonblocking` under `display` rather than `blocking` and `nonblocking` at the top level like in `embedded-hal` because the non-blocking approach is quite unique to the display driver whereas in `embedded-hal` it's a standard non-blocking approach across the board.

## Introduce simplified non-blocking API
Introduce a struct providing a simplified non-blocking API that is similar to the blocking API as an easier stepping stone towards the tiny-led-matrix.

The simplified API does come with some performance costs. Most notably the fact that the frame setting is done inside the `show` method and therefore inside the critical section. This caused slightly more flickering in debug but had no visible effect in release mode. Throughout the docs I have pointed to the instructions and example for using tiny-led-matrix directly if performance becomes a concern.

Co-authored-by: Rob Young <[email protected]>

Author: bors[bot]

CHANGELOG.md

@@ -7,7 +7,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
-(no changes)
+- Rearrange LED display modules under the same root module and change their
+  APIs to be more aligned with each other.
 
 ## [0.10.1] - 2021-05-25
 

README.md

@@ -39,12 +39,12 @@ run one of the commands below.
 
 *For micro:bit V1*
 ```bash
-> cargo run --release --manifest-path ./examples/led-blocking/Cargo.toml --features v1 --target thumbv6m-none-eabi
+> cargo run --release --manifest-path ./examples/display-blocking/Cargo.toml --features v1 --target thumbv6m-none-eabi
 ```
 
 *For micro:bit V2*
 ```bash
-> cargo run --release --manifest-path ./examples/led-blocking/Cargo.toml --features v2 --target thumbv7em-none-eabihf
+> cargo run --release --manifest-path ./examples/display-blocking/Cargo.toml --features v2 --target thumbv7em-none-eabihf
 ```
 
 You should see a lot of build output, the orange LED on the back of the micro:bit should flash quickly and

examples/led-blocking/Cargo.toml renamed to examples/display-blocking/Cargo.toml

@@ -1,5 +1,5 @@
 [package]
-name = "led-blocking"
+name = "display-blocking"
 version = "0.1.0"
 edition = "2018"
 

examples/led-blocking/src/main.rs renamed to examples/display-blocking/src/main.rs

@@ -14,7 +14,7 @@ use microbit::{
 #[cfg(feature = "v2")]
 use microbit::hal::gpio::p1::Parts as P1Parts;
 
-use microbit::led;
+use microbit::display::blocking::Display;
 
 #[entry]
 fn main() -> ! {
@@ -36,7 +36,7 @@ fn main() -> ! {
         };
 
         // Display
-        let mut leds = led::Display::new(pins);
+        let mut display = Display::new(pins);
 
         #[allow(non_snake_case)]
         let letter_I = [
@@ -91,13 +91,13 @@ fn main() -> ! {
             [0, 0, 1, 0, 0],
         ];
         loop {
-            leds.display(&mut timer, letter_I, 1000);
-            leds.display(&mut timer, heart, 1000);
-            leds.display(&mut timer, letter_R, 1000);
-            leds.display(&mut timer, letter_u, 1000);
-            leds.display(&mut timer, letter_s, 1000);
-            leds.display(&mut timer, letter_t, 1000);
-            leds.clear();
+            display.show(&mut timer, letter_I, 1000);
+            display.show(&mut timer, heart, 1000);
+            display.show(&mut timer, letter_R, 1000);
+            display.show(&mut timer, letter_u, 1000);
+            display.show(&mut timer, letter_s, 1000);
+            display.show(&mut timer, letter_t, 1000);
+            display.clear();
             timer.delay_ms(250_u32);
         }
     }

examples/led-nonblocking/Cargo.toml renamed to examples/display-nonblocking/Cargo.toml

@@ -1,5 +1,5 @@
 [package]
-name = "led-nonblocking"
+name = "display-nonblocking"
 version = "0.1.0"
 edition = "2018"
 

examples/led-nonblocking/src/main.rs renamed to examples/display-nonblocking/src/main.rs

@@ -10,9 +10,8 @@ use cortex_m::peripheral::Peripherals;
 use cortex_m_rt::entry;
 
 use microbit::{
-    display::{self, image::GreyscaleImage, Display, Frame, MicrobitDisplayTimer, MicrobitFrame},
+    display::nonblocking::{Display, GreyscaleImage},
     display_pins,
-    gpio::DisplayPins,
     hal::{
         gpio::p0::Parts as P0Parts,
         rtc::{Rtc, RtcInterrupt},
@@ -37,11 +36,8 @@ fn heart_image(inner_brightness: u8) -> GreyscaleImage {
 // We use TIMER1 to drive the display, and RTC0 to update the animation.
 // We set the TIMER1 interrupt to a higher priority than RTC0.
 
-static LED_PINS: Mutex<RefCell<Option<DisplayPins>>> = Mutex::new(RefCell::new(None));
+static DISPLAY: Mutex<RefCell<Option<Display<TIMER1>>>> = Mutex::new(RefCell::new(None));
 static ANIM_TIMER: Mutex<RefCell<Option<Rtc<RTC0>>>> = Mutex::new(RefCell::new(None));
-static DISPLAY_TIMER: Mutex<RefCell<Option<MicrobitDisplayTimer<TIMER1>>>> =
-    Mutex::new(RefCell::new(None));
-static DISPLAY: Mutex<RefCell<Option<Display<MicrobitFrame>>>> = Mutex::new(RefCell::new(None));
 
 #[entry]
 fn main() -> ! {
@@ -51,35 +47,32 @@ fn main() -> ! {
         while p.CLOCK.events_lfclkstarted.read().bits() == 0 {}
         p.CLOCK.events_lfclkstarted.reset();
 
+        // RTC at 16Hz (32_768 / (2047 + 1))
+        // 62.5ms period
+        let mut rtc0 = Rtc::new(p.RTC0, 2047).unwrap();
+        rtc0.enable_event(RtcInterrupt::Tick);
+        rtc0.enable_interrupt(RtcInterrupt::Tick, None);
+        rtc0.enable_counter();
+
+        // Set up pins
+        #[cfg(feature = "v1")]
+        let pins = {
+            let p0parts = P0Parts::new(p.GPIO);
+            display_pins!(p0parts)
+        };
+
+        #[cfg(feature = "v2")]
+        let pins = {
+            let p0parts = P0Parts::new(p.P0);
+            let p1parts = P1Parts::new(p.P1);
+            display_pins!(p0parts, p1parts)
+        };
+
+        let display = Display::new(p.TIMER1, pins);
+
         cortex_m::interrupt::free(move |cs| {
-            // RTC at 16Hz (32_768 / (2047 + 1))
-            // 62.5ms period
-            let mut rtc0 = Rtc::new(p.RTC0, 2047).unwrap();
-            rtc0.enable_event(RtcInterrupt::Tick);
-            rtc0.enable_interrupt(RtcInterrupt::Tick, None);
-            rtc0.enable_counter();
-
-            let mut timer = MicrobitDisplayTimer::new(p.TIMER1);
-
-            // Set up pins
-            #[cfg(feature = "v1")]
-            let mut pins = {
-                let p0parts = P0Parts::new(p.GPIO);
-                display_pins!(p0parts)
-            };
-
-            #[cfg(feature = "v2")]
-            let mut pins = {
-                let p0parts = P0Parts::new(p.P0);
-                let p1parts = P1Parts::new(p.P1);
-                display_pins!(p0parts, p1parts)
-            };
-
-            display::initialise_display(&mut timer, &mut pins);
-            *LED_PINS.borrow(cs).borrow_mut() = Some(pins);
+            *DISPLAY.borrow(cs).borrow_mut() = Some(display);
             *ANIM_TIMER.borrow(cs).borrow_mut() = Some(rtc0);
-            *DISPLAY_TIMER.borrow(cs).borrow_mut() = Some(timer);
-            *DISPLAY.borrow(cs).borrow_mut() = Some(Display::new());
         });
         if let Some(mut cp) = Peripherals::take() {
             unsafe {
@@ -99,20 +92,15 @@ fn main() -> ! {
 #[interrupt]
 fn TIMER1() {
     cortex_m::interrupt::free(|cs| {
-        if let Some(timer) = DISPLAY_TIMER.borrow(cs).borrow_mut().as_mut() {
-            if let Some(pins) = LED_PINS.borrow(cs).borrow_mut().as_mut() {
-                if let Some(d) = DISPLAY.borrow(cs).borrow_mut().as_mut() {
-                    display::handle_display_event(d, timer, pins);
-                }
-            }
+        if let Some(display) = DISPLAY.borrow(cs).borrow_mut().as_mut() {
+            display.handle_display_event();
         }
     });
 }
 
 #[interrupt]
 unsafe fn RTC0() {
     static mut STEP: u8 = 0;
-    static mut FRAME: MicrobitFrame = MicrobitFrame::const_default();
 
     cortex_m::interrupt::free(|cs| {
         if let Some(rtc) = ANIM_TIMER.borrow(cs).borrow_mut().as_mut() {
@@ -126,11 +114,9 @@ unsafe fn RTC0() {
         _ => unreachable!(),
     };
 
-    FRAME.set(&heart_image(inner_brightness));
-
     cortex_m::interrupt::free(|cs| {
-        if let Some(d) = DISPLAY.borrow(cs).borrow_mut().as_mut() {
-            d.set_frame(&FRAME);
+        if let Some(display) = DISPLAY.borrow(cs).borrow_mut().as_mut() {
+            display.show(&heart_image(inner_brightness));
         }
     });
 

examples/led-rtic/Cargo.toml renamed to examples/display-rtic/Cargo.toml

@@ -1,5 +1,5 @@
 [package]
-name = "led-rtic"
+name = "display-rtic"
 version = "0.1.0"
 edition = "2018"
 

examples/led-rtic/src/main.rs renamed to examples/display-rtic/src/main.rs

@@ -11,9 +11,8 @@ use defmt_rtt as _;
 use panic_halt as _;
 
 use microbit::{
-    display::{self, image::GreyscaleImage, Display, Frame, MicrobitDisplayTimer, MicrobitFrame},
+    display::nonblocking::{Display, GreyscaleImage},
     display_pins,
-    gpio::DisplayPins,
     hal::{
         gpio::p0::Parts as P0Parts,
         rtc::{Rtc, RtcInterrupt},
@@ -40,10 +39,8 @@ fn heart_image(inner_brightness: u8) -> GreyscaleImage {
 #[app(device = microbit::pac, peripherals = true)]
 const APP: () = {
     struct Resources {
-        display_pins: DisplayPins,
-        display_timer: MicrobitDisplayTimer<pac::TIMER1>,
+        display: Display<pac::TIMER1>,
         anim_timer: Rtc<pac::RTC0>,
-        display: Display<MicrobitFrame>,
     }
 
     #[init]
@@ -52,6 +49,7 @@ const APP: () = {
 
         // Starting the low-frequency clock (needed for RTC to work)
         p.CLOCK.tasks_lfclkstart.write(|w| unsafe { w.bits(1) });
+
         while p.CLOCK.events_lfclkstarted.read().bits() == 0 {}
         p.CLOCK.events_lfclkstarted.reset();
 
@@ -62,46 +60,35 @@ const APP: () = {
         rtc0.enable_interrupt(RtcInterrupt::Tick, None);
         rtc0.enable_counter();
 
-        let mut timer = MicrobitDisplayTimer::new(p.TIMER1);
-
         // Set up pins
         #[cfg(feature = "v1")]
-        let mut pins = {
+        let pins = {
             let p0parts = P0Parts::new(p.GPIO);
             display_pins!(p0parts)
         };
 
         #[cfg(feature = "v2")]
-        let mut pins = {
+        let pins = {
             let p0parts = P0Parts::new(p.P0);
             let p1parts = P1Parts::new(p.P1);
             display_pins!(p0parts, p1parts)
         };
 
-        display::initialise_display(&mut timer, &mut pins);
+        let display = Display::new(p.TIMER1, pins);
 
         init::LateResources {
-            display_pins: pins,
-            display_timer: timer,
             anim_timer: rtc0,
-            display: Display::new(),
+            display,
         }
     }
 
-    #[task(binds = TIMER1, priority = 2,
-           resources = [display_timer, display_pins, display])]
-    fn timer1(mut cx: timer1::Context) {
-        display::handle_display_event(
-            &mut cx.resources.display,
-            cx.resources.display_timer,
-            cx.resources.display_pins,
-        );
+    #[task(binds = TIMER1, priority = 2, resources = [display])]
+    fn timer1(cx: timer1::Context) {
+        cx.resources.display.handle_display_event();
     }
 
-    #[task(binds = RTC0, priority = 1,
-           resources = [anim_timer, display])]
+    #[task(binds = RTC0, priority = 1, resources = [anim_timer, display])]
     fn rtc0(mut cx: rtc0::Context) {
-        static mut FRAME: MicrobitFrame = MicrobitFrame::const_default();
         static mut STEP: u8 = 0;
 
         cx.resources.anim_timer.reset_event(RtcInterrupt::Tick);
@@ -112,9 +99,8 @@ const APP: () = {
             _ => unreachable!(),
         };
 
-        FRAME.set(&heart_image(inner_brightness));
         cx.resources.display.lock(|display| {
-            display.set_frame(FRAME);
+            display.show(&heart_image(inner_brightness));
         });
 
         *STEP += 1;