-
Notifications
You must be signed in to change notification settings - Fork 86
Add SdCardSpiDevice 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
base: develop
Are you sure you want to change the base?
Conversation
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. |
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. |
Co-authored-by: Jonathan 'theJPster' Pallant <[email protected]>
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.
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. |
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:
|
Personally I don't have the need for a generic |
|
||
- `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 |
There was a problem hiding this comment.
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.
Ok, fair enough. |
Is this still planned? I wrote a patch to fix the build test checks and open a PR in @avsaase fork. I'm interested in trying to have the async API merged, so if you need help I will be happy to contribute! |
fix rename error
Some (positive) life events happened so I couldn't work on this for a few weeks but I'm still planning on getting this merged. The last thing I did was to quickly whip up an async version of this PR to see how that would work. Unfortunately it did not work at all and consistently gave CRC errors. I haven't had the chance yet to see if this was a problem with this PR or my quick async version of it (it doesn't look like I pushed that code to github). In any case it's probably best to test the code from this PR first to see if the basic approach is correct. If you want to help with that that would be great. |
I could test your 2 versions async and blocking next week if you want I would have access to real hardware (rp2040 + some SD cards). I will just need the async code. I'm not sure what is the best way to include the async code since the other PR is still waiting. |
I pushed my async compatible version of this PR here: https://github.com/avsaase/embedded-sdmmc-rs/tree/bisync-sd-card-device. |
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.