Skip to content

Commit

Permalink
sdcard: auto-detection of insertion and delete 'remove sd card' feature
Browse files Browse the repository at this point in the history
It seems that big share of the sd card support tickets are customers not clicking the upper right corner after inserting their sd card.
That's why auto-detection of sd card in the device is critical.

With this commit, when 'insert sd card' option is called from api,
BitBox02 device will start listening its sd card port and automatically
recognize if an sd card is inserted.

Furthermore, we noticed that 'remove sd card' feature is not used in any
of our libraries. That's why we removed it from api and Python library.

Signed-off-by: asi345 <inanata15@gmail.com>
  • Loading branch information
asi345 committed Aug 29, 2024
1 parent e0e9742 commit c9db4c8
Show file tree
Hide file tree
Showing 12 changed files with 42 additions and 59 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ customers cannot upgrade their bootloader, its changes are recorded separately.
- Bitcoin: UX improvements for payment request confirmations
- Ethereum: display the addresses on device in the same case as the user input
- Allow exiting the screen asking to insert the microSD card
- SD card: Remove API to prompt removal of the microSD card
- SD card: add auto-detection of insertion

### 9.19.0
- Display device name on screen before unlock
Expand Down
3 changes: 3 additions & 0 deletions py/bitbox02/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## [Unreleased]

# 8.0.0
- SD card: Remove API to prompt removal of the microSD card from the device

# 7.0.0
- get_info: add optional device initialized boolean to returned tuple
- eth_sign: add address_case field, which should be initialized by the client
Expand Down
10 changes: 0 additions & 10 deletions py/bitbox02/bitbox02/bitbox02/bitbox02.py
Original file line number Diff line number Diff line change
Expand Up @@ -648,16 +648,6 @@ def insert_sdcard(self) -> None:
)
self._msg_query(request, expected_response="success")

def remove_sdcard(self) -> None:
# pylint: disable=no-member
request = hww.Request()
request.insert_remove_sdcard.CopyFrom(
bitbox02_system.InsertRemoveSDCardRequest(
action=bitbox02_system.InsertRemoveSDCardRequest.REMOVE_CARD
)
)
self._msg_query(request, expected_response="success")

def root_fingerprint(self) -> bytes:
"""
Get the root fingerprint from the bitbox02
Expand Down
6 changes: 0 additions & 6 deletions py/send_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ def backup_sd() -> None:
return
print("Backup created sucessfully")
print("Please Remove SD Card")
self._device.remove_sdcard()

def backup_mnemonic() -> None:
if self._device.version < semver.VersionInfo(9, 13, 0):
Expand Down Expand Up @@ -284,7 +283,6 @@ def _restore_backup_workflow(self) -> None:
eprint("Restoring backup failed")
return
print("Please Remove SD Card")
self._device.remove_sdcard()

def _restore_from_mnemonic(self) -> None:
try:
Expand Down Expand Up @@ -317,9 +315,6 @@ def _insert_sdcard(self) -> None:
except UserAbortException:
print("Aborted by user")

def _remove_sdcard(self) -> None:
self._device.remove_sdcard()

def _get_root_fingerprint(self) -> None:
print(f"Root fingerprint: {self._device.root_fingerprint().hex()}")

Expand Down Expand Up @@ -1390,7 +1385,6 @@ def _menu_init(self) -> None:
("Reboot into bootloader", self._reboot),
("Check if SD card inserted", self._check_sd_presence),
("Insert SD card", self._insert_sdcard),
("Remove SD card", self._remove_sdcard),
("Toggle BIP39 Mnemonic Passphrase", self._toggle_mnemonic_passphrase),
("Retrieve Ethereum xpub", self._get_eth_xpub),
("Retrieve Ethereum address", self._display_eth_address),
Expand Down
14 changes: 5 additions & 9 deletions src/rust/bitbox02-rust/src/hww/api/sdcard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,15 @@ pub async fn process(
&pb::InsertRemoveSdCardRequest { action }: &pb::InsertRemoveSdCardRequest,
) -> Result<Response, Error> {
let inserted = bitbox02::sd::sdcard_inserted();
let action = match SdCardAction::try_from(action) {
Ok(action) => action,
Err(_) => return Ok(Response::Success(pb::Success {})),
match SdCardAction::try_from(action) {
Ok(SdCardAction::InsertCard) => {},
_ => return Ok(Response::Success(pb::Success {})),
};
// No action required, already inserted (INSERT request) or not inserted (REMOVE request)
if (action == SdCardAction::InsertCard && inserted)
|| (action == SdCardAction::RemoveCard && !inserted)
if inserted
{
return Ok(Response::Success(pb::Success {}));
}
sdcard::sdcard(action == SdCardAction::InsertCard).await?;
sdcard::sdcard().await?;
Ok(Response::Success(pb::Success {}))
}

Expand Down Expand Up @@ -75,7 +73,6 @@ mod tests {
// insert
mock(Data {
sdcard_inserted: Some(false),
ui_sdcard_create_arg: Some(true),
..Default::default()
});
assert_eq!(
Expand All @@ -88,7 +85,6 @@ mod tests {
// remove
mock(Data {
sdcard_inserted: Some(true),
ui_sdcard_create_arg: Some(false),
..Default::default()
});
assert_eq!(
Expand Down
4 changes: 2 additions & 2 deletions src/rust/bitbox02-rust/src/workflow/sdcard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ use core::cell::RefCell;

pub struct UserAbort;

pub async fn sdcard(insert: bool) -> Result<(), UserAbort> {
pub async fn sdcard() -> Result<(), UserAbort> {
let result = RefCell::new(None as Option<Result<(), UserAbort>>);
let mut component = bitbox02::ui::sdcard_create(insert, |sd_done| {
let mut component = bitbox02::ui::sdcard_create(|sd_done| {
*result.borrow_mut() = if sd_done {
Some(Ok(()))
} else {
Expand Down
2 changes: 0 additions & 2 deletions src/rust/bitbox02/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ use crate::keystore;
pub struct Data {
pub ui_confirm_create: Option<Box<dyn Fn(&super::ui::ConfirmParams) -> bool>>,
pub sdcard_inserted: Option<bool>,
pub ui_sdcard_create_arg: Option<bool>,
pub ui_transaction_address_create: Option<Box<dyn Fn(&str, &str) -> bool>>,
pub ui_transaction_fee_create: Option<Box<dyn Fn(&str, &str, bool) -> bool>>,
pub ui_trinary_input_string_create:
Expand All @@ -40,7 +39,6 @@ unsafe impl Sync for SafeData {}
pub static DATA: SafeData = SafeData(RefCell::new(Data {
ui_confirm_create: None,
sdcard_inserted: None,
ui_sdcard_create_arg: None,
ui_transaction_address_create: None,
ui_transaction_fee_create: None,
ui_trinary_input_string_create: None,
Expand Down
3 changes: 1 addition & 2 deletions src/rust/bitbox02/src/ui/ui.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ where
}
}

pub fn sdcard_create<'a, F>(insert: bool, callback: F) -> Component<'a>
pub fn sdcard_create<'a, F>(callback: F) -> Component<'a>
where
// Callback must outlive component.
F: FnMut(bool) + 'a,
Expand All @@ -214,7 +214,6 @@ where

let component = unsafe {
bitbox02_sys::sdcard_create(
insert,
Some(c_callback::<F>),
// passed to the C callback as `param`
Box::into_raw(Box::new(callback)) as *mut _,
Expand Down
4 changes: 1 addition & 3 deletions src/rust/bitbox02/src/ui/ui_stub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,12 +93,10 @@ where
}
}

pub fn sdcard_create<'a, F>(insert: bool, mut callback: F) -> Component<'a>
pub fn sdcard_create<'a, F>(mut callback: F) -> Component<'a>
where
F: FnMut(bool) + 'a,
{
let data = crate::testing::DATA.0.borrow();
assert_eq!(data.ui_sdcard_create_arg.unwrap(), insert);
callback(true);
Component {
is_pushed: false,
Expand Down
2 changes: 1 addition & 1 deletion src/rust/bitbox02/src/ui/ui_stub_c_unit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ where
}
}

pub fn sdcard_create<'a, F>(_insert: bool, mut callback: F) -> Component<'a>
pub fn sdcard_create<'a, F>(mut callback: F) -> Component<'a>
where
F: FnMut(bool) + 'a,
{
Expand Down
49 changes: 26 additions & 23 deletions src/ui/components/sdcard.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,39 @@
#include <hardfault.h>
#include <screen.h>
#include <sd.h>
#include <string.h>
#include <touch/gestures.h>
#include <ui/screen_stack.h>

typedef struct {
// if true, the callback won't be called until the sd card is inserted.
// the insert/remove label changes depending on this flag.
bool insert;
void (*callback)(bool, void*);
void* callback_param;
// TODO: use a TIMER interrupt to get a more accurate timer.
// 250 is ~0.5 sec. Unit: rendering rate.
int check_interval;
int count;
} data_t;

static void _insert_poll_callback(component_t* component)
{
data_t* data = (data_t*)component->data;
if (data->callback && sd_card_inserted()) {
data->callback(true, data->callback_param);
data->callback = NULL;
return;
}
}

static void _render(component_t* component)
{
image_sdcard(screen_is_upside_down());
ui_util_component_render_subcomponents(component);

data_t* data = (data_t*)component->data;
if (data->count == data->check_interval) {
data->count = 0;
_insert_poll_callback(component);
}
data->count++;
}

/********************************** Component Functions **********************************/
Expand All @@ -47,17 +64,6 @@ static const component_functions_t _component_functions = {

/********************************** Create Instance **********************************/

static void _continue_callback(component_t* component)
{
data_t* data = (data_t*)component->parent->data;
if (!data->insert || sd_card_inserted()) {
if (data->callback) {
data->callback(true, data->callback_param);
data->callback = NULL;
}
}
}

static void _cancel_callback(component_t* component)
{
data_t* data = (data_t*)component->parent->data;
Expand All @@ -67,7 +73,7 @@ static void _cancel_callback(component_t* component)
}
}

component_t* sdcard_create(bool insert, void (*callback)(bool, void*), void* callback_param)
component_t* sdcard_create(void (*callback)(bool, void*), void* callback_param)
{
component_t* component = malloc(sizeof(component_t));
if (!component) {
Expand All @@ -80,9 +86,10 @@ component_t* sdcard_create(bool insert, void (*callback)(bool, void*), void* cal
memset(data, 0, sizeof(data_t));
memset(component, 0, sizeof(component_t));

data->insert = insert;
data->callback = callback;
data->callback_param = callback_param;
data->check_interval = 250;
data->count = 0;
component->data = data;
component->f = &_component_functions;
component->dimension.width = SCREEN_WIDTH;
Expand All @@ -91,15 +98,11 @@ component_t* sdcard_create(bool insert, void (*callback)(bool, void*), void* cal
ui_util_add_sub_component(
component,
label_create(
insert ? "Insert SD card\nto continue" : "Remove SD card\nto continue",
"Insert SD card\nto continue",
NULL,
screen_is_upside_down() ? RIGHT_CENTER : LEFT_CENTER,
component));
ui_util_add_sub_component(
component, icon_button_create(top_slider, ICON_BUTTON_CHECK, _continue_callback));
if (insert) {
ui_util_add_sub_component(
component, icon_button_create(top_slider, ICON_BUTTON_CROSS, _cancel_callback));
}
component, icon_button_create(top_slider, ICON_BUTTON_CROSS, _cancel_callback));
return component;
}
2 changes: 1 addition & 1 deletion src/ui/components/sdcard.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,6 @@
* @param[in] insert if true, the user is asked to insert the sdcard. Otherwise the user is asked to
* remove it.
*/
component_t* sdcard_create(bool insert, void (*callback)(bool, void*), void* callback_param);
component_t* sdcard_create(void (*callback)(bool, void*), void* callback_param);

#endif

0 comments on commit c9db4c8

Please sign in to comment.