rust-embedded-community / ssd1306

SSD1306 OLED driver
Apache License 2.0
283 stars 69 forks source link

trait for disp.flush() #152

Closed pdgilbert closed 3 years ago

pdgilbert commented 3 years ago

I have a function with code for refreshing a display so my loop is easier to read. This extracted simplified example works

Click to expand ``` // display function for blue pill stm32f103 #![deny(unsafe_code)] #![no_std] #![no_main] #[cfg(debug_assertions)] use panic_semihosting as _; #[cfg(not(debug_assertions))] use panic_halt as _; use cortex_m_rt::entry; use core::fmt::Write; use embedded_graphics::{ fonts::{Font8x16, Text}, //Font6x8, pixelcolor::BinaryColor, prelude::*, style::{TextStyle, TextStyleBuilder}, }; use ssd1306::{prelude::*, Builder, I2CDIBuilder}; use stm32f1xx_hal::{ delay::Delay, device::I2C2, i2c::{BlockingI2c, DutyCycle, Mode, Pins}, pac::{CorePeripherals, Peripherals}, prelude::*, }; fn setup() -> (BlockingI2c>, Delay) { let cp = CorePeripherals::take().unwrap(); let p = Peripherals::take().unwrap(); let mut rcc = p.RCC.constrain(); let clocks = rcc.cfgr.freeze(&mut p.FLASH.constrain().acr); let mut gpiob = p.GPIOB.split(&mut rcc.apb2); // for i2c scl and sda // can have (scl, sda) using I2C1 on (PB8, PB9 ) or on (PB6, PB7) // or (scl, sda) using I2C2 on (PB10, PB11) let i2c = BlockingI2c::i2c2( p.I2C2, ( gpiob.pb10.into_alternate_open_drain(&mut gpiob.crh), // scl on PB10 gpiob.pb11.into_alternate_open_drain(&mut gpiob.crh), // sda on PB11 ), //&mut afio.mapr, need this for i2c1 but not i2c2 Mode::Fast { frequency: 100_000.hz(), duty_cycle: DutyCycle::Ratio2to1, }, clocks, &mut rcc.apb1, 1000, 10, 1000, 1000, ); let delay = Delay::new(cp.SYST, clocks); (i2c, delay) // return tuple (i2c, delay) } fn display( bat_mv: i16, bat_ma: i16, load_ma: i16, temp_c: i16, values_b: [i16; 3], disp: &mut impl DrawTarget, //disp : impl DrawTarget + stm32f1xx_hal::prelude::_embedded_hal_serial_Write, text_style: TextStyle, ) -> () { let mut lines: [heapless::String; 4] = [ heapless::String::new(), heapless::String::new(), heapless::String::new(), heapless::String::new(), ]; write!(lines[0], "bat:{:4}mV{:4}mA", bat_mv, bat_ma).unwrap(); write!(lines[1], "load: {:5}mA", load_ma).unwrap(); write!( lines[2], "B:{:4} {:4} {:4}", values_b[0], values_b[1], values_b[2] ) .unwrap(); write!(lines[3], "temperature{:3} C", temp_c).unwrap(); let _z = disp.clear(BinaryColor::Off); // check for err variant for i in 0..lines.len() { let _z = Text::new(&lines[i], Point::new(0, i as i32 * 16)) .into_styled(text_style) .draw(&mut *disp); // check for err variant } //disp.flush().unwrap(); //works when this is moved outside display() () } #[entry] fn main() -> ! { let (i2c, mut delay) = setup(); let manager = shared_bus::BusManager::, _>::new(i2c); let interface = I2CDIBuilder::new().init(manager.acquire()); let mut disp: GraphicsMode<_, _> = Builder::new() .size(DisplaySize128x64) // set display size 128x32, 128x64 .connect(interface) .into(); disp.init().unwrap(); let text_style = TextStyleBuilder::new(Font8x16) .text_color(BinaryColor::On) .build(); Text::new("Display initialized ...", Point::zero()) .into_styled(text_style) .draw(&mut disp) .unwrap(); disp.flush().unwrap(); let mut count = 0i16; loop { let (bat_mv, bat_ma, load_ma, temp_c, values_b) = (3300, 5, 10, 21, [100, 200, 300]); display( bat_mv, bat_ma, load_ma, temp_c, values_b, &mut disp, text_style, ); disp.flush().unwrap(); // works here but need write trait to move inside display() count = count + 1; delay.delay_ms(2000_u16); // sleep for 2s } } ```

but if I move disp.flush() into the display() function where it really belongs then I get the compiling problem

105 |     disp.flush().unwrap();  //works when this is moved outside display()
    |          ^^^^^ method not found in `&mut impl DrawTarget<BinaryColor>`
    |
    = help: items from traits can only be used if the type parameter is bounded by the trait
help: the following trait defines an item `flush`, perhaps you need to restrict type parameter `impl DrawTarget<BinaryColor>` with it:
    |
76  |     disp: &mut impl DrawTarget<BinaryColor> + stm32f1xx_hal::prelude::_embedded_hal_serial_Write,
    |                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

My attempts to add the Write part of the trait have not been successful, I need a type argument that I have not yet guessed. But it seems strange that it is asking for serial_Write when the device is on i2c. What am I missing? Suggestions on how to specify this would be appreciated.

jamwaffles commented 3 years ago

Hey, thanks for the detailed report and apologies for the delay. On the off chance you haven't figured out a solution yet, here's something that might help. I had a quick play around and come up with this function:

fn display(disp: &mut GraphicsMode<impl WriteOnlyDataCommand, impl DisplaySize>) -> () {
    let style = PrimitiveStyleBuilder::new()
        .stroke_width(1)
        .stroke_color(BinaryColor::On)
        .build();

    Rectangle::new(Point::new(0, 0), Point::new(127, 63))
        .into_styled(style)
        .draw(disp)
        .unwrap();

    disp.flush().unwrap();

    ()
}

GraphicsMode is a concrete type that provides the flush() method - no traits required. I'm not sure where you got the _embedded_hal_serial_Write import suggestion (IDE auto complete getting confused maybe?) but it's not required. GraphicsMode takes some generics which I'm using impl trait syntax for above. I did a quick test just drawing a rectangle which compiles fine.


Please note that I've just released ssd1306 version 0.6.0 which restructures some of the crate interface so the following code will only work with 0.6.0 and above. You'll also need to upgrade to embedded-graphics 0.7 which includes more breaking changes. The above function would look like this now:

fn draw<S>(display: &mut Ssd1306<impl WriteOnlyDataCommand, S, BufferedGraphicsMode<S>>)
where
    S: DisplaySize,
{
    let style = PrimitiveStyleBuilder::new()
        .stroke_width(1)
        .stroke_color(BinaryColor::On)
        .build();

    Rectangle::new(Point::new(0, 0), Size::new(127, 63))
        .into_styled(style)
        .draw(display)
        .unwrap();

    display.flush().unwrap();
}

I moved S: DisplaySize out into a type parameter because it should be the same type between Ssd1306 and BufferedGraphicsMode.

pdgilbert commented 3 years ago

I'm a bit confused. My display() function was called in the loop to write the data onto the screen. disp and text_style were initialized before and passed as arguments along with the data to display()

display(
            bat_mv, bat_ma, load_ma, temp_c, values_b, &mut disp, text_style,
        );

Yours seems to be doing the initialization I did before the loop but not using the data? Am I missing something? How do I call your display function and pass data to it?

pdgilbert commented 3 years ago

With the new version I'm also having trouble finding the concrete type GraphicsMode. Where is it exported from now?

jamwaffles commented 3 years ago

Yours seems to be doing the initialization I did before the loop but not using the data? Am I missing something? How do I call your display function and pass data to it?

Sorry if that wasn't clear - you can add all your arguments back in. I removed them for the sake of brevity/clarity as I only wanted to demonstrate the type required for disp.

With the new version I'm also having trouble finding the concrete type GraphicsMode. Where is it exported from now?

GraphicsMode was renamed to BufferedGraphicsMode to open the door to supporting immediate/other rendering modes in the future.

pdgilbert commented 3 years ago

Sorry I should have seen some of that, but was getting confused by something else I had wrong too. Using the new crate release, the version below now works with disp.flush() pulled into the display() function. One minor difference is that I had to shift the Text::new() write down by 10 so the first line is completely on the screen. Perhaps there is a better way to shift this? (Also, I do not understand why the first line is yellow while others are blue, but this is not a change.)

Click to expand ``` // display function for blue pill stm32f103 #![deny(unsafe_code)] #![no_std] #![no_main] #[cfg(debug_assertions)] use panic_semihosting as _; #[cfg(not(debug_assertions))] use panic_halt as _; use cortex_m_rt::entry; use core::fmt::Write; use embedded_graphics::{ mono_font::{ascii::FONT_8X13, MonoTextStyle, MonoTextStyleBuilder, }, //FONT_6X10 FONT_8X13 pixelcolor::BinaryColor, prelude::*, text::{Baseline, Text}, }; use ssd1306::{prelude::*, I2CDisplayInterface, Ssd1306, mode::BufferedGraphicsMode}; use stm32f1xx_hal::{ delay::Delay, device::I2C2, i2c::{BlockingI2c, DutyCycle, Mode, Pins}, pac::{CorePeripherals, Peripherals}, prelude::*, }; fn setup() -> (BlockingI2c>, Delay) { let cp = CorePeripherals::take().unwrap(); let p = Peripherals::take().unwrap(); let mut rcc = p.RCC.constrain(); let clocks = rcc.cfgr.freeze(&mut p.FLASH.constrain().acr); let mut gpiob = p.GPIOB.split(&mut rcc.apb2); // for i2c scl and sda let i2c = BlockingI2c::i2c2( p.I2C2, ( gpiob.pb10.into_alternate_open_drain(&mut gpiob.crh), // scl on PB10 gpiob.pb11.into_alternate_open_drain(&mut gpiob.crh), // sda on PB11 ), //&mut afio.mapr, need this for i2c1 but not i2c2 Mode::Fast { frequency: 100_000.hz(), duty_cycle: DutyCycle::Ratio2to1, }, clocks, &mut rcc.apb1, 1000, 10, 1000, 1000, ); let delay = Delay::new(cp.SYST, clocks); (i2c, delay) // return tuple (i2c, delay) } fn display( bat_mv: i16, bat_ma: i16, load_ma: i16, temp_c: i16, values_b: [i16; 3], text_style: MonoTextStyle, disp: &mut Ssd1306>) -> () where S: DisplaySize, { let mut lines: [heapless::String<32>; 4] = [ heapless::String::new(), heapless::String::new(), heapless::String::new(), heapless::String::new(), ]; write!(lines[0], "bat:{:4}mV{:4}mA", bat_mv, bat_ma ).unwrap(); write!(lines[1], "load: {:5}mA", load_ma ).unwrap(); write!(lines[2], "B:{:4} {:4} {:4}", values_b[0], values_b[1], values_b[2]).unwrap(); write!(lines[3], "temperature{:3} C", temp_c ).unwrap(); disp.clear(); for i in 0..lines.len() { // shift down by 10 so first line is on display Text::new(&lines[i], Point::new(0, i as i32 * 16 + 10), text_style).draw(&mut *disp).unwrap(); } disp.flush().unwrap(); } #[entry] fn main() -> ! { let (i2c, mut delay) = setup(); let manager = shared_bus::BusManager::, _>::new(i2c); let interface = I2CDisplayInterface::new(manager.acquire()); let mut disp = Ssd1306::new(interface, DisplaySize128x64, DisplayRotation::Rotate0) .into_buffered_graphics_mode(); disp.init().unwrap(); let text_style = MonoTextStyleBuilder::new() .font(&FONT_8X13) // &FONT_6X10 &FONT_8X13 .text_color(BinaryColor::On) .build(); Text::with_baseline("Display init...", Point::zero(), text_style, Baseline::Top ) .draw(&mut disp) .unwrap(); disp.flush().unwrap(); let mut count = 0i16; loop { let (bat_mv, bat_ma, load_ma, temp_c, values_b) = (3300, 5, 10, 21, [100, 200, 300]); display(bat_mv, bat_ma, load_ma, temp_c, values_b, text_style, &mut disp ); count = count + 1; delay.delay_ms(2000_u16); // sleep for 2s } } ```

Beware that for bluepill I am using PR https://github.com/stm32-rs/stm32f1xx-hal/pull/340 because the setup() function uses traits in a simplified way that has not yet been merged into the main branch. A more complete version of the example, called battery_monitor_ads1015, is also building (with several other examples) on several different HAL crates with my CI at https://github.com/pdgilbert/rust-integration-testing/actions but I have not run tested on hardware.

jamwaffles commented 3 years ago

Great to hear you got it working!

One minor difference is that I had to shift the Text::new() write down by 10 so the first line is completely on the screen.

embedded-graphics 0.7 sets the text origin point to the font's baseline by default now. Take a look at the bottom of this section in the migration guide for an example on how to set the origin back to top left if you need to do that.

Also, I do not understand why the first line is yellow while others are blue, but this is not a change.

Many SSD1306 modules have a yellow strip at the top of the display, I guess for use as a status bar in MP3 players or whatnot. If you look for white or blue modules, they're usually a single colour.

Also, e-g 0.7 supports multiline text now so you don't need to use an array of heapless::String; adding \n where you need it should work fine if you want to simplify your code a little.

It looks like your problem is solved, so I'll close this ticket but please reopen it if you're still getting stuck!

pdgilbert commented 3 years ago

Thanks for all your help.