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

sd: prompt user for formatting if format is not VFAT compatible #1330

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ customers cannot upgrade their bootloader, its changes are recorded separately.
- Update manufacturer HID descriptor to bitbox.swiss
- Ethereum: remove deprecated Goerli network
- SD card: solve backup bug when sd card is re-inserted
- SD card: prompt user for formatting if sd is not VFAT compatible

### 9.21.0
- Bitcoin: add support for sending to silent payment (BIP-352) addresses
Expand Down
18 changes: 18 additions & 0 deletions src/rust/bitbox02-rust/src/hww/api/backup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,24 @@ pub async fn create(
})
.await?;

let is_vfat_formatted = bitbox02::sd::sdcard_vfat_formatted();
if !is_vfat_formatted {
confirm::confirm(&confirm::Params {
title: "SD card\nformatting needed",
body: "This will erase all\ndata on the SD card.",
..Default::default()
})
.await?;
confirm::confirm(&confirm::Params {
title: "WARNING!",
body: "PERMANENT DATA\nLOSS IF NOT\nBACKED UP",
longtouch: true,
..Default::default()
})
.await?;
bitbox02::sd::format()?;
}

let is_initialized = bitbox02::memory::is_initialized();

if is_initialized {
Expand Down
1 change: 1 addition & 0 deletions src/rust/bitbox02-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ const ALLOWLIST_FNS: &[&str] = &[
"screen_saver_disable",
"screen_saver_enable",
"sd_card_inserted",
"sd_card_vfat_formatted",
"sd_erase_file_in_subdir",
"sd_format",
"sd_free_list",
Expand Down
17 changes: 17 additions & 0 deletions src/rust/bitbox02/src/sd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,23 @@ pub fn sdcard_inserted() -> bool {
data.sdcard_inserted.unwrap()
}

#[cfg(not(feature = "testing"))]
pub fn sdcard_vfat_formatted() -> bool {
unsafe { bitbox02_sys::sd_card_vfat_formatted() }
}

#[cfg(feature = "testing")]
pub fn sdcard_vfat_formatted() -> bool {
true
}

pub fn format() -> Result<(), ()> {
match unsafe { bitbox02_sys::sd_format() } {
true => Ok(()),
false => Err(()),
}
}

struct SdList(bitbox02_sys::sd_list_t);

impl Drop for SdList {
Expand Down
10 changes: 8 additions & 2 deletions src/sd.c
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,14 @@ bool sd_erase_file_in_subdir(const char* fn, const char* subdir)
return status;
}

#ifdef TESTING
bool sd_card_vfat_formatted(void)
{
memset(&fs, 0, sizeof(FATFS));
FRESULT res = f_mount(&fs, "", 1);
f_unmount("");
return res != FR_NO_FILESYSTEM;
}

bool sd_format(void)
{
const MKFS_PARM params = {
Expand All @@ -384,4 +391,3 @@ bool sd_format(void)
uint8_t work[FF_MAX_SS] = {0};
return f_mkfs("SD", &params, work, sizeof(work)) == FR_OK;
Comment on lines 391 to 392
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is FF_MAX_SS, and what does this param do in general?

Is this suitable for production or was it possibly only a good value for the tests/simulator? Asking b/c I am not familiar, but the sd_format function was originally added for unit tests only.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This set of options defines the extent of sector size used for the low level disk I/O interface, disk_read and disk_write function. Valid values are 512, 1024, 2048 and 4096. FF_MIN_SS defines minimum sector size and FF_MAX_SS defines the maximum sector size. Always set both 512 for memory card and harddisk.

Taken from FatFs configurations, http://elm-chan.org/fsw/ff/doc/config.html#max_ss. Only thing that boggled me was "SD" parameter there, but I did not encounter any issue while testing out. The card resetted and continued to work without an error.

}
#endif
3 changes: 1 addition & 2 deletions src/sd.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,7 @@ USE_RESULT bool sd_write_bin(
uint16_t length,
bool replace);

#ifdef TESTING
USE_RESULT bool sd_card_vfat_formatted(void);
USE_RESULT bool sd_format(void);
#endif

#endif