feature: add sd card manager support recovering from sd card getting disconnected #94
Conversation
| pub fn is_mounted(mut self) { | ||
| // Sd crate doesn't have a check method for SD card being still mounted | ||
| // Use `card_size_bytes()` as indicator if device is still connected or not | ||
| match self.sd_controller.device().card_size_bytes() { |
There was a problem hiding this comment.
@NoahSprenger reading the card size is the best option I could find from the sd crate to check if the device is connected.
This method does get called for every health check there's likely a performance penalty.
Do we want another alternative for how to check sd is connected? Or maybe just instead of health checking we can only send an error message when the sd manager is called to read/write it.
| } | ||
| } | ||
| pub fn write( | ||
| &mut self, | ||
| file: &mut sd::File, | ||
| buffer: &[u8], | ||
| ) -> Result<usize, sd::Error<sd::SdMmcError>> { | ||
| if !self.is_mounted() { | ||
| return sd::Error::DeviceError(()); |
There was a problem hiding this comment.
@NoahSprenger Since this method is expecting sd::Error<sd::SdMmcError>> what type of error do you think I should throw there? Or silently ignore?
| let mut root_directory: Option<Directory> = None; | ||
| if volume.is_some() { | ||
| root_directory = match sd_controller.open_root_dir(volume.as_ref().unwrap()) { | ||
| Ok(root_directory) => Some(root_directory), | ||
| Err(_) => { | ||
| error!("Cannot get root"); | ||
| None | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| let file: Result<sd::File, sd::Error<sd::SdMmcError>>; | ||
| if volume.is_some() && root_directory.is_some() { | ||
| file = sd_controller.open_file_in_dir( | ||
| volume.as_mut().unwrap(), | ||
| root_directory.as_ref().unwrap(), | ||
| "log.txt", | ||
| sd::Mode::ReadWriteCreateOrTruncate, | ||
| ); | ||
| } |
There was a problem hiding this comment.
@NoahSprenger what's a more rusty way of doing this? There's a chain of dependencies for these Option variables volume -> root -> file.
There was a problem hiding this comment.
Are nested matches ok? Been trying to avoid those but if that's the rusty way
| } | ||
| self.sd_controller.close_dir( | ||
| self.volume.as_ref().unwrap(), | ||
| self.root_directory.as_ref().unwrap() |
There was a problem hiding this comment.
@NoahSprenger So sd_controller.close_dir for some reason doesn't take a &Directory dir as the second param but Directory dir unwrapping without as_ref throws a different error
cannot move out of `self.root_directory` which is behind a mutable reference
help: consider calling `.as_ref()` or `.as_mut()` to borrow the type's contents
There was a problem hiding this comment.
If I don't make the root_directory mutable then I can't do the pattern I did up there where I initialize the root_directory after I checked to see volume is already setup. I feel like there's a more rusty way of doing that pattern which would likely fix this as well
SInce it's assigning None first then the actual Directory once the check is successful
| let file: Result<sd::File, sd::Error<sd::SdMmcError>>; | ||
| if volume.is_some() && root_directory.is_some() { | ||
| file = sd_controller.open_file_in_dir( | ||
| volume.as_mut().unwrap(), |
There was a problem hiding this comment.
@NoahSprenger Since there are checks for the optionals being populated the unwraps should be guaranteed to not panic but is there a safer way to handle this?
Same question for when unwrap is used down in the other methods
|
I think it'd be better for us to sit down together to go over this PR. |
No description provided.