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

ESP32 SDCard assumes that the card detect pin going low means a card is inserted #1475

Closed
CoryCharlton opened this issue Apr 23, 2024 · 6 comments · Fixed by nanoframework/System.IO.FileSystem#110 or nanoframework/nf-interpreter#3008

Comments

@CoryCharlton
Copy link
Member

Target name(s)

ESP_*

Firmware version

No response

Was working before? On which version?

No response

Device capabilities

No response

Description

https://github.com/nanoframework/nf-interpreter/blob/6a56d849be58368837175494608cbcbb60e11220/targets/ESP32/_nanoCLR/System.IO.FileSystem/nf_sys_io_filesystem_nanoFramework_System_IO_FileSystem_SDCard.cpp#L11

The existing code assumes that if the card detect pin is low then that means an SD card is inserted. However I have two SD card modules from Adafruit that go high to signal that a card is inserted (https://www.adafruit.com/product/4682 | https://www.adafruit.com/product/254).

How to reproduce

No response

Expected behaviour

The correct events should fire based on the physical state of the SD card.

Screenshots

No response

Aditional information

I'm not sure if this is as simple as flipping the existing logic. ie: are there modules that use a low pin value to indicate a card is inserted?

Another alternative would be to add a field to the SDCardMmcParameters and SDCardSpiParameters in order to configure the correct pin value. My concern is how/if this would impact non-ESP32 targets. My initial check looks like only ESP32 devices support SDCard but I've only worked on ESP32 devices in nanoFramework so I might be overlooking something.

@Ellerbach
Copy link
Member

Related to #1447 for the alternative way. Which indeed should be the best because with the ESP32 family, we now have various pins depending on the platform. So, it should be parameterized.

@CoryCharlton
Copy link
Member Author

I'm happy to work on this but it looks like nanoframework/nf-interpreter#2829 is close so I'll wait for that to get merged first.

@josesimoes
Copy link
Member

This requires some thinking about the feature design. So far we've been trying (and succedded) in NOT having platform specific stuff in the "general" class libraries. The platform specifics are all in the various nanoFramework.NNNN.
Worth give it some though before we decide how to implement this.

@CoryCharlton
Copy link
Member Author

@Ellerbach I'm not sure that this is directly related to #1447.

@josesimoes was your comment meant for #1447?

This doesn't appear to be platform specific but rather specific to the hardware implementation. We're assuming that the card detect pin has a mode of InputPullUp. However there is nothing preventing the user from using an InputPullDown pin.

Additionally I don't believe that we can infer what LOW or HIGH represent with relation to the physical card state as this input doesn't necessarily even need to be directly related to SD card slot.

@josesimoes
Copy link
Member

@CoryCharlton you are correct. I've added the above comment thinking about #1447... 🤦🏻‍♂️
Despite that it's also valid for the detect pin.

Adding to the above, but I get it that this could be a bit more tricky to implement, I would like to see for ESP32 the same implementation that the STM32 devices have: the card detection GPIO is handled internally, and the mounting takes place auto-magically after insertion. This frees the developer from the hassle of having to programatically call the mount operation, which is something that's most obviously going to happen. Note that the storage device inserted event is still fired, of course.

@CoryCharlton
Copy link
Member Author

CoryCharlton commented Apr 26, 2024

@josesimoes I'm not clear how the SDCard is handled in the STM32 devices as I'm unable to find an implementation of nanoFramework.System.IO.FileSystem.SDCard in any target other than ESP32.

I'm assuming that SDCard is not used in the STM32 devices and instead the drive is just automatically mounted like you mention (I think I see evidence of that here.

If that's the case then I think the STM32 implementation is incorrect (or at least doesn't cover all use cases). A card detect pin does not need to be attached in order for an SD card to function. I have several SD card modules that do not even expose a card detect pin.

Even for the modules that do expose a card detect pin I often don't bother wasting a GPIO for it because my use cases have always required that an SD card be available. I handle this during application initialization and fail if it does not mount.

Long story short we can't rely on card detect for mounting. SDCard could be extended to auto-mount but I think it should still be configurable as there may be use cases where manual mounting is preferred.

I'll start a conversation in Discord to get more input into this. Link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment