This PR tries to address a few things with a major refactor:

1. Fixes #31 by forcing init before use via typestates
2. Simplifies construction of Display via the builder pattern and unifies all options into one place
3. Adds framebuffer > display size default offset handling logic
4. Removes unnecessary RST reset pin from Display itself into the builder's init function*

Together these 3 changes should make things more straight forward while removing a set of possible bugs if users called things like set_orientation before calling init. It also simplifies the Model by removing the ModelOptions ownership .

• NOTE - this change technically allows abuse of the reset pin outside of the display's control since we do not consume it, only use it during init call. This means that someone could change that pin to other state, or send a reset signal in the middle of the display doing something. I consider this an extreme edge case not worth the "consume" logic that'd be needed here and additional generic param propagation + release logic.

Introduces DisplayOptions with all possible MADCTL values abstracted into a more digestible struct.

I've also refactored Orientation to include inversion logic so we don't have init vs set_orientation asymmetry.

This should fix #10

~This is kind of weird:~

• ~white is correct~
• ~red is correct~
• ~blue is correct~
• ~green is correct~
• ~magenta comes correct (red + blue)~
• ~yellow comes out correct (red + green) ~
• blue comes out correct from arduino app

~But with this library:~

• ~blue comes out as red.~
• ~cyan comes out as yellow (green + blue).~

LATE: Sorry, disregard most of the above. I tested again, red comes out as blue and blue comes out as red. It does work correctly with arduino.

This seems to be a clear case of red and blue being swapped. But green remains the same.

It is consistent. Filling screen fills it entirely with the same colour. Happens for all draw operations.

With this change:

diff --git a/Cargo.toml b/Cargo.toml
index 8026834..9e1d54e 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -15,6 +15,7 @@ display-interface = "0.4.1"
 embedded-graphics-core = "0.3.3"
 embedded-hal = "0.2.6"
 nb = "1.0.0"
+log = "0.4"
 
 [dependencies.heapless]
 optional = true
diff --git a/src/models/ili9486.rs b/src/models/ili9486.rs
index 4c1475b..cbdb2ae 100644
--- a/src/models/ili9486.rs
+++ b/src/models/ili9486.rs
@@ -96,9 +96,11 @@ impl Model for ILI9486Rgb666 {
     {
         write_command(di, Instruction::RAMWR, &[])?;
         let mut iter = colors.into_iter().flat_map(|c| {
+            use log::*;
             let red = c.r() << 2;
             let green = c.g() << 2;
             let blue = c.b() << 2;
+            error!("ddd {red:?} {green:?} {blue:?}");
             [red, green, blue]
         });

I get a lot of:

1970-01-01T00:06:11.158Z ERROR mipidsi::models::ili9486                > ddd 0 0 252

This looks like the correct BLUE colour to me. But this was displayed as RED.

So no idea what is going on here. But it does that something weird is going on with the colours.

So maybe the hardware has swapped the red and blue lines? But strange the arduino app works.

Logging this as a new bug (although in a way I suspect it might be my own fault). I apolagize in advance if I am misisng some small thing, and this is more a request for help than a bug report

I do have an ESP32 with the st7789 attached to it. Cirtcuit configuration can be found here: https://raw.githubusercontent.com/Fri3dCamp/badge-2020/master/media/Badge_Block.png

Some (setup) example code with Arduino can be found here https://github.com/Fri3dCamp/Badge2020_arduino/blob/main/Badge2020_TFT.h / https://github.com/Fri3dCamp/Badge2020_arduino This code example works (just as sanity check that my hardware is working)

I am pretty sure my pin config is correct (5 for CS, 33 for DC and 26 for RST), as configured in example code from before. (Note 26 does not appear on the schema, maybe it does not have RST, but I tried it with st7789_without_rst as well.., also in working arduino code it has been set as wel

In all cases I just get a black screen and I am not sure why as I expect a cleaned red screen I notice with this I have to configure my own SPI config which is somehow "magically"? handled when using Arduino/Adadruit library so maybe I am doing something wrong here... (I took inspiration from https://github.com/mutantbob/lcd-esp32)

  # Tried both MODE_3 and MODE_0
    let config = <esp_idf_hal::spi::config::Config as Default>::default().data_mode(embedded_hal::spi::MODE_3);
    let spi = esp_idf_hal::spi::Master::<esp_idf_hal::spi::SPI2, _, _, _, esp_idf_hal::gpio::Gpio23<esp_idf_hal::gpio::Unknown>>::new(
        peripherals.spi2,
        esp_idf_hal::spi::Pins {
            sclk: peripherals.pins.gpio18,
            sdo: peripherals.pins.gpio19,
            sdi: Some(peripherals.pins.gpio23),
            cs: None,
        },
        config,
    )
    .unwrap();
    let lcd_dc = peripherals.pins.gpio33.into_output().unwrap();
    let lcd_rst = peripherals.pins.gpio26.into_output().unwrap();
    let lcd_cs = peripherals.pins.gpio5.into_output().unwrap();
    let mut led_bl = peripherals.pins.gpio12.into_output().unwrap();
    led_bl.set_high();
    let di = display_interface_spi::SPIInterface::new(spi, lcd_dc, lcd_cs);
    println!("SPI CONFIG DONE");


    let mut lcd = mipidsi::Display::st7789(di, lcd_rst);
    let mut delay = esp_idf_hal::delay::Ets;
    println!("INIT S");

    lcd.init(
        &mut delay,
        mipidsi::DisplayOptions {
            orientation: mipidsi::Orientation::PortraitInverted(false),
            invert_vertical_refresh: false,
            color_order: Default::default(),
            invert_horizontal_refresh: false,
        },
    ).unwrap();


    println!("INITED");

    lcd.clear(Rgb565::RED);
    println!("CLEAR");

   # I am joining on another thread here to make sure my program does not exit

I'm using a 128x160 ST7735 display and I'm noticing that the colors appear to be inverted.

display.clear(Rgb565::WHITE); makes the whole display black, display.set_pixels(1, 1, 10, 10, core::iter::repeat(Rgb565::YELLOW).take(100)); draws a 10x10 red square display.set_pixel(11, 11, Rgb565::RED); draws a single yellow pixel at coordinate (11, 11)

I think that I am setting up the display correctly, below is the code used to create the display as shown in the attached image.

fn main() -> anyhow::Result<()> {
    // Temporary. Will disappear once ESP-IDF 4.4 is released, but for now it is necessary to call this function once,
    // or else some patches to the runtime implemented by esp-idf-sys might not link properly.
    esp_idf_sys::link_patches();

    let peripherals = Peripherals::take().unwrap();

    let sdo = peripherals.pins.gpio7;
    let sclk = peripherals.pins.gpio6;
    //let cs = peripherals.pins.gpio10;
    let a0 = peripherals.pins.gpio8.into_output()?; // dc
    let rst = peripherals.pins.gpio9.into_output()?;

    let spi = Master::<SPI2, _, _, Gpio2<Unknown>, Gpio10<Unknown>>::new(
        peripherals.spi2,
        Pins {
            sclk,
            sdo,
            sdi: None,
            cs: None,
        },
        //Config::default().baudrate(20_000_000.Hz()),
        Config {
            baudrate: 60_000_000.Hz(),
            write_only: true,
            ..Default::default()
        },
    )?;

    let di = display_interface_spi::SPIInterfaceNoCS::new(spi, a0);
    let mut display = mipidsi::Builder::st7735s(di)
        .with_display_size(128, 160)
        .with_framebuffer_size(128, 160)
        .with_color_order(mipidsi::ColorOrder::Rgb)
        .with_orientation(mipidsi::Orientation::Portrait(false))
        .init(&mut Ets, Some(rst))
        .unwrap();

    display.clear(Rgb565::WHITE);

    display.set_pixels(1, 1, 10, 10, core::iter::repeat(Rgb565::YELLOW).take(100));
    display.set_pixel(11, 11, Rgb565::RED);

    Ok(())
}

For some reason, using mipidsi with textboxes from embedded-text seems to produce a weird cut-off issue if the display has a pretty harsh offset. To preface, I'm unsure whether this issue is with mipidsi or with embedded-text, but it's probably best that the issue is tracked somewhere.

About the setup: I'm using an Adafruit Feather ESP32-S3 (TFT), with esp-hal. The TFT attached to the board is a 240x135 ST7789, with the SKU ADA4383. From my testing with a filled box and a contrasting stroke colour, it seems to have similar offsets to the pico1 model, which is why that's used in my code below (along with mipidsi 0.5.0).

Text just appears to get cut-off at a seemingly arbitrary point on the display, no matter what. I have tried extending the text box, changing rotations, using different display models (in the code, not hardware), starting points, height modes, etc. None of them seem to allow text to be printed over this arbitrary cut-off point.

let di = SPIInterfaceNoCS::new(interface, dc);

let mut display = Builder::st7789_pico1(di)
    .with_display_size(240, 135)
    .with_orientation(Orientation::Landscape(true))
    .init(Delay::get_delay_mut(), Some(rst))
    .unwrap();

display.clear(Rgb565::BLACK).unwrap();

let text_style = MonoTextStyleBuilder::new()
    .font(&FONT_9X15)
    .text_color(Rgb565::WHITE)
    .build();

let textbox_style = TextBoxStyleBuilder::new()
    .alignment(HorizontalAlignment::Center)
    .vertical_alignment(VerticalAlignment::Middle)
    .build();

let bounds = Rectangle::new(Point::zero(), Size::new(235, 130));

let text_box = TextBox::with_textbox_style(text, bounds, text_style, textbox_style);

text_box.draw(display).unwrap();

The text below is supposed to say "1Player", but it doesn't show further than the P. It's not dead-centre as this was an attempt while I tried to use a wider textbox, but this issue persists no matter what.

Solid colours still fill as expected, and below is a picture of a red fill (apologies for the bad photo, it is red). This box is using the exact same bounds as the textbox, just with a solid red fill.

This PR tries to address #27 and #20 by refactoring the DisplayOptions and init paths so variants of the same Display and Model can be used with different physical display sizes.

This also adds support for "cropped" display variants in which the display framebuffer is bigger but the visible display is smaller as well as "offset" from the (0, 0) coordinates internally.

This PR adds the ILI9341 model, that was previously added as part of #39. I've cleaned up the code some more and extracted the common code for the ILI3941 and ILI9342C controllers into a separate module.

~~One remaining issue is the color order: Before #36 the RGB/BGR bit for the ILI9342C was inverted. According to the datasheet this shouldn't be necessary, but without the inversion I get incorrect colors on my display. I'm not sure if the datasheet is wrong or what is going on, but I've checked that my display is indeed using the RGB subpixel order by using a magnifying glass.~~

Fix #1

So with the other changes I was able to test and validate this setup on both TTGO Display and TTGO TWatch-2020 v1.

Anyway I admit, I'm not sure to understand every commands of the init seq, so do not hesitate to correct me if something is odd.

It seems part of the screen is not drawn on when using PortraitInverted or LandscapeInverted, as if the image is outside the screen. Though it's fine using Portrait or Landscape.

I am using st7789_240x240 method since that is the resolution of the display.

I will contact our provider and try to get the exact part number and datasheet. It's supposed to be a st789v2 with 240x240 pixels.

Here are images of the behavior

Some screens require different madctl 0x26 settings, and e.g. will appear reversed without.

For a possible solution see: https://github.com/yuri91/ili9341-rs/commit/8a4aee95c2934cd6aaac4b243ab8bda24d0b7475

Although I tend to wonder if maybe this solution is overkill. All that is required I think is for the set_orientation function to take a parameter for the required madctl parameter, and an orientation parameter so it can work out the size (see #8).

I tried a lot, but I couldn't find a solution about the colors.

Rgb565::WHITE -> Outputs black Rgb565::BLACK -> Outputs white

My code:

    let spi = spi::Spi::new(
        peripherals.SPI2,
        io.pins.gpio4,   
        io.pins.gpio5,   
        io.pins.gpio25,   
        io.pins.gpio13,   
        100u32.kHz(),
        SpiMode::Mode0, 
        &mut system.peripheral_clock_control,
        &clocks,
    );

    let reset = io.pins.gpio26.into_push_pull_output();
    let di = SPIInterfaceNoCS::new(spi, io.pins.gpio15.into_push_pull_output());

    let mut display = mipidsi::Builder::st7735s(di)
        .with_display_size(128,160)
        .with_color_order(ColorOrder::Rgb) // Even if I did BGR the problem didn't go away
        .init(&mut delay, Some(reset))
        .unwrap();
    display.clear(Rgb565::BLACK).unwrap();

The default_options for some models contain with_invert_colors(true) and others don't. IMO it would be better to be consistent across all models and default to non inverted colors.

The decision if color inversion is necessary is entirely dependent on the LCD glass and not the controller model and can only be decided for a specific LCD module. For LCD module specific constructors, like the st7789_pico1, the with_invert_colors(true) should stay. To help users who want to use other LCD modules there should be additional docs to help them identify problems, as also mentioned in #47.

The default color order for some display was changed in #36, but this hasn't been documented in the changelog or the migration notes.

This change will probably affect many users, so that it would also be a good idea to add a troubleshooting section for common color problems to the docs. Some example images and/or directions how to identify RGB/BGR or color inversion on TN vs IPS displays would be very useful IMO.

At the moment the DrawTarget::clear implementation clears the entire framebuffer, even for smaller display. The unnecessary writes makes clearing the display slower than necessary on these displays. I would suggest to change the implementation of clear or simply to remove it and rely on the default implementation, that uses fill_solid.

I'm not sure if it is ever necessary to clear the entire framebuffer, but if that is the case we could add another method clear_all to clear the entire framebuffer.

Adds DCS abstraction and a new Dcs wrapper on top of display-interface to:

1. Ensure correct and safe use of DCS commands (where applicable)
2. Make init code for models more readable
3. Separate the DCS aspects out to another crate (in a separate PR)