Exponential backoff for timeout and Rotation functionality.

[kolupaev]

Hi,

Was using your library for M5EPD, and it worked perfectly, except 2 minor features:

• Frequent timeouts on screen busy-wait, along with, I believe, free-rtos freezes
• Functionality to rotate the screen

Anyhow, here is a pull request. I've tried to minimize modifications to the public interfaces, however to prevent extra copying needed to make some. However I don't think it will break any code.

I took liberty to add a bit more logging to simplify debugging, which is disabled by default (via log-max-level).

Rotation itself ported from M5EPD driver (see https://github.com/m5stack/M5EPD/blob/main/src/M5EPD_Driver.cpp#L368 )

Example use:

    let epd: M5Display = IT8951::new_with_mcs(
        display_interface,
        Config::default(),
        MemoryConverterSetting {
            rotation: it8951::memory_converter_settings::MemoryConverterRotation::Rotate90,
            ..Default::default()
        },
    )
    .init(2300)
    .expect("Unable to initialize display");

    let refresh_box = { 
      // Use as normal via embedded_graphics
      // fill_contiguous is rotated automatically by it8951 controller
    };
    // refresh_box is rotated by driver code 
    display.display_area(refresh_box)?;
    

Code is tested on M5EPD (0, and 90 rotation), and works as expected.

[pbert519]

Hi,

rotation is a great feature, thanks for implementing.

We do have timeout issues on the busy wait too, but no solution yet. (on esp32c3 with esp32-hal)
From my initial analysis, it looks like the IT8951 itself freezes.
A possible next step would be to do a full compare of all communication against a working c-library, but time :(
For our usecase, a full reset of the it8951 is enough.

I'll not be able to test your code till next thursday.

The CI shows some clippy warnings, could you fix them?

[kolupaev]

We do have timeout issues on the busy wait too, but no solution yet. (on esp32c3 with esp32-hal)
From my initial analysis, it looks like the IT8951 itself freezes.

Interesting, never noticed that on mine, sleep/wakeup cycle works fine as well. However, at times takes up to ~600ms.

I (1273) app::hardware: Initializing display
I (1276) gpio: GPIO[27]| InputEn: 0| OutputEn: 0| OpenDrain: 0| Pullup: 0| Pulldown: 0| Intr:0
I (1700) it8951::interface: Total delay ~335800 us
I (2129) it8951::interface: Total delay ~6200 us
I (2133) app::hardware: Initialized display: DevInfo { panel_width: 960, panel_height: 540, memory_address: 1193696, firmware_version: "5mtsca_k.v.0\01", lut_version: "M841\0T" }
I (2566) it8951::interface: Total delay ~340600 us
I (3422) it8951::interface: Total delay ~6200 us
I (3426) it8951::interface: Total delay ~3000 us
I (3430) it8951::interface: Total delay ~3000 us
...
I (7640) app::ui: Screen powered down, awaiting change
I (7861) it8951::interface: Total delay ~151800 us
I (7862) app::ui: Display awakened
I (7938) app::ui: Complete draw Rectangle { top_left: Point { x: 192, y: 251 }, size: Size { width: 208, height: 32 } }
I (7939) app::ui: Refreshing Rectangle { top_left: Point { x: 192, y: 251 }, size: Size { width: 208, height: 32 } }
I (7958) it8951::interface: Total delay ~6200 us
I (7962) it8951::interface: Total delay ~3000 us
I (7963) it8951::interface: Total delay ~600 us
I (8817) it8951::interface: Total delay ~679800 us
I (8818) app::ui: Screen powered down, awaiting change

The CI shows some clippy warnings, could you fix them?

Completely missed that clippy exists, thanks, fixed.

[pbert519]

Overall i dont think exposing MemoryConverterSettings as whole is a good approach for the normal embedded_graphics user.
It allows the user of the API to set random values for e.g. endians and bit per pixel, and the impl for embedded_graphics does not check if the settings are correctly set.

Instead we should provide a dedicated rotation setting.

[pbert519]

Yes, lets add an Rotation Enum for the Display Rotation and add it to the the config struct. Am 10. Januar 2025 18:13:12 MEZ schrieb kolupaev ***@***.***>:

…

@kolupaev commented on this pull request. > @@ -127,18 +127,43 @@ pub struct IT8951<IT8951Interface, State> { dev_info: Option<DevInfo>, marker: core::marker::PhantomData<State>, config: Config, + memory_converter_settings: MemoryConverterSetting, Good point, I'll see how separate enum would work out. Initially wanted to minimize new entities and reuse as much as possible to minimize api changes. You are onboard with a new enum for public surface API? -- Reply to this email directly or view it on GitHub: #8 (comment) You are receiving this because you commented. Message ID: ***@***.***>

[pbert519]

Looks good.
If you could run cargo fmt to make the CI happy, i will merge your changes.

[kolupaev]

Thanks, pushed a commit with format-fix.

[pbert519]

Many thanks for your contribution.
I created a new release 0.4.2 with your changes, which is available on crates.io.

[kolupaev]

Happy to help!
thank you for the library to begin with :)