Fix LCD_CAM i8080 potentially sending garbage data to display

[yanshay]

LCD_CAM i8080 sometime outputs garbage to the display.

This is due to a race condition when LCD_CAM starts sending data to display before the DMA had time to get data to the FIFO. The solution is quite simple, adding a short delay (delay based on what's used in esp-idf) between starting the DMA and activating LCD_CAM send.

For more info - #1086 (comment)

Thank you!

Thank you for your contribution.
Please make sure that your submission includes the following:

Must

• The code compiles without errors or warnings.
• All examples work.
• cargo fmt was run.
• Your changes were added to the CHANGELOG.md in the proper section.
• [N/A ] You updated existing examples or added examples (if applicable).
• [N/A] Added examples are checked in CI

Nice to have

• You add a description of your work to this PR.
• You added proper docs for your newly added features and code.

[yanshay]

I don't understand what's going on.
I added a single line of code and everything fails, not related to my change.
Will need assistance here from maintainers.

[jessebraham]

I'm not 100% sure what's happening here, I think it may be a regression in nightly, possibly related to TAIT.

It was building yesterday and nothing in the repo has changed since then. I re-ran CI on the main branch of my fork (which is up-to-date with upstream) and it is now failing too. So, fairly certain this isn't an us-problem.

With that said, it's Saturday morning and I'm not sure how deep I'll dig into this right now. 😅 I will spend at least a few minutes investigating today and update you if I do find anything, but otherwise this will likely have to wait until Monday. Sorry for the inconvenience, unfortunately this is the price we pay for living on the bleeding edge 😁

[yanshay]

From my perspective this can easily wait, enjoy the weekend.

[jessebraham]

Just to add some additional information from my (brief) investigation:

I ran cargo-expand with all the required arguments on one of the failing examples, in this case embassy_serial:

$ cd examples/
$ cargo expand --features=esp32c2,async,embassy,embassy-executor-thread,embassy-time-timg0,embassy-generic-timers --target=riscv32imc-unknown-none-elf --bin=embassy_serial

From the resulting output, we'll look at this excerpt:

#[doc(hidden)]
async fn __writer_task(
    mut tx: UartTx<'static, UART0>,
    signal: &'static Signal<NoopRawMutex, usize>,
) {
    use core::fmt::Write;
    embedded_io_async::Write::write(
            &mut tx,
            b"Hello async serial. Enter something ended with EOT (CTRL-D).\r\n",
        )
        .await
        .unwrap();
    embedded_io_async::Write::flush(&mut tx).await.unwrap();
    loop {
        let bytes_read = signal.wait().await;
        signal.reset();
        (&mut tx)
            .write_fmt(format_args!("\r\n-- received {0} bytes --\r\n", bytes_read))
            .unwrap();
        embedded_io_async::Write::flush(&mut tx).await.unwrap();
    }
}
fn writer(
    tx: UartTx<'static, UART0>,
    signal: &'static Signal<NoopRawMutex, usize>,
) -> ::embassy_executor::SpawnToken<impl Sized> {
    type Fut = impl ::core::future::Future + 'static;
    const POOL_SIZE: usize = 1;
    static POOL: ::embassy_executor::raw::TaskPool<Fut, POOL_SIZE> = ::embassy_executor::raw::TaskPool::new();
    unsafe { POOL._spawn_async_fn(move || __writer_task(tx, signal)) }
}

The first of our two build errors for this example (they're both the same root issue):

error: concrete type differs from previous defining opaque type use
  --> src/bin/embassy_serial.rs:31:1
   |
31 | #[embassy_executor::task]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^ expected `writer::Fut`, got `impl Future<Output = ()>`
   |
note: previous use here
  --> src/bin/embassy_serial.rs:31:1
   |
31 | #[embassy_executor::task]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^
   = note: this error originates in the attribute macro `embassy_executor::task` (in Nightly builds, run with -Z macro-backtrace for more info)

So, as far as I can tell (this is sort of a guess at this point), it's upset with type Fut = impl ::core::future::Future + 'static; (which is generated via the embassy-executor package). But, possibly something else is going on here.

[yanshay]

@Dominaezzz : I added the delay to fix the garbage data sent sometimes to the display. It fixed the issues for my app. Basically a single line of code.
It didn't fix the issue where the example flashes different colors in release and debug mode and I couldn't find how to fix that.

Anyway, @bjoernQ asked that someone tests this change to work (or I guess at least doesn't break the example or anything else) since he doesn't have a compatible device to test with.

Can you help by testing this so it can be merged?

Thanks

[Dominaezzz]

Hello! I'll have some time later this week to run the example again on this PR.

[jessebraham]

Sorry for the delays with fixing CI, if you rebase CI should be green now.

Also just FYI, @JurajSadel and/or @playfulFence will attempt to hunt down a devkit with an appropriate display to keep in the office, so that we can hopefully test this functionality in the future.

[Dominaezzz]

Just ran the example and it still works the same for me.

[bjoernQ]

@yanshay Really sorry for the huge delay here. I think we will need another rebase to get the "HIL Test" steps pass

[bjoernQ]

Seems like this line insists on duplicating itself

[yanshay]

I rebased, it went strange (but then I'm not a git expert), and now I see it doesn't compile in the build. Maybe I did something wrong.

But then I tried taking main and building the i8080 example and It doesn't compile either on my machine, so maybe it's not me but rather something that changed on the main branch?

I could try fixing the example but I don't want to make a mess.

[bjoernQ]

I just tried compile the example on current main and it compiled fine for me 🤔

[yanshay]

I redid the change from scratch based on main, tested, now it seems to be fine.

[bjoernQ]

LGTM

[bjoernQ]

Sorry, this took so long to get merged! Thanks!