qiuchengxuan / async-sdmmc

3 stars 2 forks source link

Improvements and CI #4

Open elpiel opened 1 year ago

elpiel commented 1 year ago
elpiel commented 1 year ago

@qiuchengxuan I fixed a few missed places for the non-async API and exported a few private members. I've also included a docsrs attributes to showcase which members/modules are enabled by a feature.

I would like to see if the CI works but I suppose you need to enable to build first or we should merge it first in main?!

qiuchengxuan commented 1 year ago

You should meld 4 commits into one

qiuchengxuan commented 1 year ago

@qiuchengxuan I fixed a few missed places for the non-async API and exported a few private members. I've also included a docsrs attributes to showcase which members/modules are enabled by a feature.

I would like to see if the CI works but I suppose you need to enable to build first or we should merge it first in main?!

Up to you, both ok to me

elpiel commented 1 year ago

@qiuchengxuan I'm trying to test a littlefs storage impl with async-sdmmc, however, I keep getting timeouts on read_block or panics.

Another thing to note: tx & rx can have difference sizes and currently the Transfer::transfer impl for SPI is panicking.

https://github.com/qiuchengxuan/async-sdmmc/blob/464c1f44b89eb5c0b537520fb8a7b6ac26c6c582/src/bus/spi/bus.rs#L42

This is especially true since Bus::tx & Bus::rx already pass empty buffers to transfer: https://github.com/qiuchengxuan/async-sdmmc/blob/464c1f44b89eb5c0b537520fb8a7b6ac26c6c582/src/bus/spi/bus.rs#L96-L102

embedded_hal::spi Transfer trait seems to return the word read in the transaction, not sure what you meant that rx & tx are the same for embedded_hal. So far I have made this solution to the issue:

#[cfg(not(any(feature = "async", feature = "async-trait")))]
impl<E, T: spi::Transfer<u8, Error = E>> Transfer for T {
    type Error = E;

    fn transfer(&mut self, tx: &[u8], rx: &mut [u8]) -> Result<(), Self::Error> {
        // when rx > tx
        if rx.len() > tx.len() {
            // only replace the tx bytes length in rx
            rx[..tx.len()].copy_from_slice(tx);

            assert_eq!(&rx[..tx.len()], tx);

        } else if rx.len() < tx.len() {
            rx.copy_from_slice(&tx[..rx.len()])
        } else {
            // when rx == tx
            rx.copy_from_slice(tx);
        }

        self.transfer(rx).map(|_| ())
    }
}
elpiel commented 1 year ago

On the Timeout issue, I've found a wrongly connected wire and managed to get to the CRC checks now.

One thing to note - I had to alter the loop for the token to continue looping if unknown token has been met.

Now, for some reason, I get a CRC error, which happens before the read_block that was failing before :thinking:

qiuchengxuan commented 1 year ago

Oops, bus.rs:97 passed an empty slice as rx while bus.rs:42 copies tx to rx

Need to replace it with embedded_hal::blocking::spi::Write when tx only

qiuchengxuan commented 1 year ago

On the Timeout issue, I've found a wrongly connected wire and managed to get to the CRC checks now.

One thing to note - I had to alter the loop for the token to continue looping if unknown token has been met.

Now, for some reason, I get a CRC error, which happens before the read_block that was failing before 🤔

In theory 74 cycles should be sufficient, and several sdmmc library use identical amount of loop, so I've no idea why it's unstable either. Actually I'm not familiar with sdmmc too

elpiel commented 1 year ago

I haven't had time to work on a fix, however, you can check a fix for such problems with initialization of the card in this branch: https://github.com/rust-embedded-community/embedded-sdmmc-rs/pull/76

It has worked very reliably so far, even though embedded-sdmmc crate has other issues related to the fat filesystem implementation