Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix build on ESP32C3 LCD Kit by upgrading HAL #74

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ottobonn
Copy link

Thanks for making this project; it's a major help to people like me trying to get into Rust programming on ESP32 while the documentation is still lacking.

I was not able to compile this code for the ESP32C3 LCD Kit due to several different errors, the most pressing of which was the fact that newer versions of the mipidsi crate require the DelayNs trait to be present on Delay, but older versions of the esp32c3-hal don't implement it.

Upgrading the HAL in various places fixes the issue for this board. I haven't addressed the other boards in this PR; this is just to show what might be required to upgrade them. I did the corresponding upgrade to the DMA display driver as well.

@@ -6,7 +6,7 @@ edition = "2021"
license = "MIT"

[target.riscv32imc-unknown-none-elf.dependencies]
hal = { package = "esp32c3-hal", version = "0.14.0" }
hal = { package = "esp-hal", version = "0.16", features = ["esp32c3", "eh1"] }
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eh1 is required for DelayNs

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was unused

esp32-c3-lcdkit/src/main.rs Outdated Show resolved Hide resolved
panic!()
}
};
let mut display = mipidsi::Builder::new(mipidsi::models::GC9A01, di)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API has changed quite a bit

Comment on lines +18 to +22
esp32 = ["esp-hal/esp32"]
esp32c3 = ["esp-hal/esp32c3"]
esp32c6 = ["esp-hal/esp32c6"]
esp32s2 = ["esp-hal/esp32s2"]
esp32s3 = ["esp-hal/esp32s3"]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Different boards are now crate features of the HAL

@georgik
Copy link
Owner

georgik commented Mar 20, 2024

@ottobonn Thank you very much for the upgrade. Amazing work!

I was waiting with upgrade until guys finished unification of HAL.
Let me do some tests and validation before merging PR.

@ottobonn
Copy link
Author

The changes I made to the DMA display driver definitely broke the other boards, so this PR isn't complete yet!

@georgik
Copy link
Owner

georgik commented Mar 22, 2024

@ottobonn I've tested the build and flash with the latest Rust. It works nicely. I've just noticed, that the project is downloading all hals, which seems to be reduntant. Other than that, the changes seems to be in the good direction.

Do you plan to update rest of boards? Just let me know, I can replicate your changes and test it with real HW for all combinations.

@ottobonn
Copy link
Author

I don't think I quite know enough about the other boards to make the changes for them, so you can go ahead! Thanks for taking a look!

@georgik
Copy link
Owner

georgik commented Apr 19, 2024

@bjoernQ Do you have any suggestion how to update esp-display-interface-spi-dma to esp-hal 0.17?

I've got half way through the compilation, but it seems that there is need for one additional generic. I would appreciate any hint. Here's PR for esp-display-interface-spi-dma: https://github.com/georgik/esp-display-interface-spi-dma/pull/1/files

@bjoernQ
Copy link

bjoernQ commented Apr 19, 2024

@bjoernQ Do you have any suggestion how to update esp-display-interface-spi-dma to esp-hal 0.17?

I've got half way through the compilation, but it seems that there is need for one additional generic. I would appreciate any hint. Here's PR for esp-display-interface-spi-dma: https://github.com/georgik/esp-display-interface-spi-dma/pull/1/files

You probably don't need to be generic over DmaMode and just use esp_hal::Blocking I guess

@georgik
Copy link
Owner

georgik commented Apr 26, 2024

During the upgrade of esp-display-interface-spi-dma we discovered a limitation of new HAL API esp-rs/esp-hal#1512 . We need to figure out a proper solution to make the upgrade possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants