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

Incorrect pin assignment for cs in sd/spi.rs on git master branch #491

Closed
jimmer74 opened this issue Oct 11, 2024 · 4 comments
Closed

Incorrect pin assignment for cs in sd/spi.rs on git master branch #491

jimmer74 opened this issue Oct 11, 2024 · 4 comments

Comments

@jimmer74
Copy link
Contributor

Hi, trying to work out why my sdcard wasn't working, I was noodling through the source (i'm using the git [pathc.crates-io] in my Cargo.toml) and found the fololowing for spi_driver::new():

  pub fn new(
       spi_driver: T,
       cs: Option<impl Peripheral<P = impl OutputPin> + 'd>,
       cd: Option<impl Peripheral<P = impl InputPin> + 'd>,
       wp: Option<impl Peripheral<P = impl InputPin> + 'd>,
       int: Option<impl Peripheral<P = impl InputPin> + 'd>,
       #[cfg(not(any(
           esp_idf_version_major = "4",
           all(esp_idf_version_major = "5", esp_idf_version_minor = "0"),
           all(esp_idf_version_major = "5", esp_idf_version_minor = "1"),
       )))] // For ESP-IDF v5.2 and later
       wp_active_high: Option<bool>,
   ) -> Result<Self, EspError>
   where
       T: Borrow<SpiDriver<'d>>,
   {
       let dev_config = sdspi_device_config_t {
           host_id: spi_driver.borrow().host(),
           gpio_cs: cs.map(|cd| cd.into_ref().deref().pin()).unwrap_or(-1), <----------- this line!!
           gpio_cd: cd.map(|cd| cd.into_ref().deref().pin()).unwrap_or(-1),
           gpio_wp: wp.map(|wp| wp.into_ref().deref().pin()).unwrap_or(-1),
           gpio_int: int.map(|int| int.into_ref().deref().pin()).unwrap_or(-1),

Appears the card detect pin is being mapped into the gpio_cs AND gpio_cd. Doesn't seem to effect functionality in my case as only sdcard is on this bus, but might stymie people with multiple spi devices?

Best, Jim

@Vollbrecht
Copy link
Collaborator

oh yeah that looks wrong, do you want to PR a quick fix?

@jimmer74
Copy link
Contributor Author

I've actually just realised it's "right" - looking at it again as the gpio.cs is mapped as|cd| in the closure, so it's actually dereffing the right pin anyway, just the captured variable name is wrong!

I've been looking at misbehaving code all day, now it's all starting to look naughty to me!

@Vollbrecht
Copy link
Collaborator

yeah its "right" that is technically correct but well it's still a "typo" overall ;P

@jimmer74
Copy link
Contributor Author

#492

1st one I've done, so hoping it's right!

@github-project-automation github-project-automation bot moved this from Todo to Done in esp-rs Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

No branches or pull requests

2 participants