Skip to content

Add SdCardDevice trait #185

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

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

avsaase
Copy link

@avsaase avsaase commented Apr 10, 2025

Closes #184.

This only adds a blocking trait. Once #176 is merged I can rebase and add the async version. Or this can be merged first and the other PR can add the async version. Either is fine with me.

Cs,
}

impl<BUS, CS> SdCardDevice for (&RefCell<BUS>, CS)
Copy link
Author

@avsaase avsaase Apr 10, 2025

Choose a reason for hiding this comment

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

I couldn't add an impl for (BUS, CS) because it conflicted with this one.

@thejpster
Copy link
Member

thejpster commented Apr 11, 2025

Seems reasonable enough, but I'd like to see some current users of embedded-sdmmc-rs port over to this style to check it's usable in practice.

@thejpster
Copy link
Member

There's also some overlap with #170, which is about SD Cards in general (i.e. using SD Card Host Controllers with the 4-bit bus) rather than this which is about SPI bus based SD Cards specifically.

avsaase and others added 2 commits April 11, 2025 19:44
Co-authored-by: Jonathan 'theJPster' Pallant <[email protected]>
@avsaase
Copy link
Author

avsaase commented Apr 11, 2025

Seems reasonable enough, but I'd like to see some current users of embedded-sdmmc-rs port over to this style to check it's usable in practice.

I'll also do some dogfooding by using it in my project. I'm mostly interested in the async API but I think I can do some tests with the block API.

There's also some overlap with #170, which is about SD Cards in general (i.e. using SD Card Host Controllers with the 4-bit bus) rather than this which is about SPI bus based SD Cards specifically.

How do you want to move forward with this? There hasn't been any recent activity on that PR. I think it makes sense to first fix the dummy byte issue.

@thejpster
Copy link
Member

If we're going to abstract the SD card transport it makes sense to do it in a way that both fixes the SPI issue and allows the use of SD Host Controllers.

I also have a mild preference for custom types over implementing traits on tuples. I think it makes it clearer what's going on.

Maybe something like:

  • struct VolumeManager uses a BlockDevice
  • SdCard implements BlockDevice using an SdCardTransport
  • RefCellSpiSdCardTransport implements SdCardTransport using a &RefCell
  • Stm32SdHostTransport also implements SdCardTransport
  • etc

@avsaase
Copy link
Author

avsaase commented Apr 12, 2025

Personally I don't have the need for a generic Transport trait. I understand why it would be useful to have but I think that's a separate thing from what this PR tries to a do. If @luojia65 is still interested in their PR I'm happy to discuss how we can combine efforts but I would like to limit the scope of this PR to just fixing the dummy byte issue.


- `embedded-hal-bus-03`: adds support for `embedded-hal-bus::spi::ExclusiveDevice`. This is probably the easiest way to use this crate when the SD card is the only device on the bus.
- `embassy-sync-06`: adds support for using a blocking mutex from the `embassy-sync` crate. This allows sharing the bus between multiple tasks.

```rust
use embedded_sdmmc::{SdCard, VolumeManager, Mode, VolumeIdx};
// Build an SD Card interface out of an SPI device, a chip-select pin and the delay object
Copy link
Member

Choose a reason for hiding this comment

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

This example needs to be kept in sync with the copy in readme-test.

@thejpster
Copy link
Member

Ok, fair enough.

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.

Sending 0xFF byte with CS deasserted when used on shared bus
2 participants