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

Prelude improvements/fixes #860

Merged
merged 3 commits into from
Oct 20, 2023
Merged

Conversation

jessebraham
Copy link
Member

As discussed previously with @bjoernQ and @MabezDev:

  • Move the SPI traits into their own preludes in the spi::master/spi::slave modules
  • Remove the embedded-hal-async trait re-exports
  • Remove the eh module, and in turn the [email protected] trait re-exports

I will deal with embedded-hal in a following PR, then I think we should be okay here.

Copy link
Contributor

@JurajSadel JurajSadel left a comment

Choose a reason for hiding this comment

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

LGTM!
Just a small thing, regarding CI, there are a few unused import warnings in some examples.

@jessebraham
Copy link
Member Author

Thanks for the heads up! Will get those errors resolved before the next release.

Copy link
Contributor

@bjoernQ bjoernQ left a comment

Choose a reason for hiding this comment

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

LGTM

@MabezDev
Copy link
Member

Would it be possible to convert the wonky examples (that were previously held back by the prelude)? I think it's just embassy_spi and embassy_serial.

e.g

diff --git a/esp32-hal/examples/embassy_serial.rs b/esp32-hal/examples/embassy_serial.rs
index 40d1031..c5dfe23 100644
--- a/esp32-hal/examples/embassy_serial.rs
+++ b/esp32-hal/examples/embassy_serial.rs
@@ -29,13 +29,10 @@ const AT_CMD: u8 = 0x04;
 #[embassy_executor::task]
 async fn writer(mut tx: UartTx<'static, UART0>) {
     esp_println::println!("writing...");
-    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();
+    tx.write(b"Hello async serial. Enter something ended with EOT (CTRL-D).\r\n")
+        .await
+        .unwrap();
+    tx.flush().await.unwrap();
 }
 
 #[embassy_executor::task]
@@ -47,7 +44,7 @@ async fn reader(mut rx: UartRx<'static, UART0>) {
 
     let mut rbuf: Vec<u8, MAX_BUFFER_SIZE> = Vec::new();
     let mut offset = 0;
-    while let Ok(len) = embedded_io_async::Read::read(&mut rx, &mut rbuf[offset..]).await {
+    while let Ok(len) = rx.read(&mut rbuf[offset..]).await {
         offset += len;
         if offset == 0 {
             rbuf.truncate(0);

@jessebraham
Copy link
Member Author

Oh yes of course, I had forgotten about those. Will get that resolved and rebase this branch!

@jessebraham
Copy link
Member Author

jessebraham commented Oct 20, 2023

Actually it looks like I will need to hold off on fixing these until I remove the [email protected] traits from the prelude and make the other necessary fixes. We're still have conflicting functions because of these, unfortunately.

I have rebased to resolve the merge conflicts, though.

@jessebraham jessebraham merged commit 33bfe80 into esp-rs:main Oct 20, 2023
17 checks passed
@jessebraham jessebraham deleted the fixes/prelude branch October 20, 2023 14:06
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.

4 participants