sd-card: Increase usb processing timeout if sd card presence is checked. #1624
Conversation
Some users experienced a bug where they got "backup creation failed" even though the backup was successfully created. This issue could be reproduced by forcing `sd_card_inserted` to always take 1s. The logs showed that the usb processing incorrectly timed out. The processing timeout is therefore reset to about 1s more than the default. For good measure the loop checking the sd card presence is also changed to iterate more times and wait shorter times. One common case is that it runs a single iteration of the loop, and that is now much quicker.
| { | ||
| // The sd card is allowed up to 1s to initialize, therefore the usb processing timeout must also | ||
| // be increased by about 1s. | ||
| usb_processing_timeout_reset(-10); |
There was a problem hiding this comment.
Did you check if we actually sleep in case of backup create, assuming the app checked sd card presence beforehand once (which it does)? So we can more confidently release a hotfix knowing it's the right fix. Maybe one of the other operations in backup creation (copy_seed, writing backup) also causes delays and we should give more time there?
There was a problem hiding this comment.
I checked now and during backup creation it only sleeps once, either if the sd-card was inserted before the device was started or after a replug of the sd-card.
There was a problem hiding this comment.
When I create a backup from the app it calls sd_card_inserted() 88 times
There was a problem hiding this comment.
during the device initialization it didn't sleep during the backup creation for me. But I don't think that is conclusive either way.
There was a problem hiding this comment.
Maybe sleeping in sd_card_inserted() isn't the culprit, but the timeout_reset still solves the issue because sd_card_inserted() is called from many places.
There was a problem hiding this comment.
If the culprit is not sd_card_inserted (which it probably is not if there is no sleep and the check is fast), then the timeout reset should not be here right?
How about measuring how long backup.rs:create() takes, roughly?
When I create a backup from the app it calls sd_card_inserted() 88 times
That is crazy and clearly a bug, but can you specify when it is called? The app can't make API calls during another API call, so if it's the app making all these calls, it should technically not interfere with backup creation.
There was a problem hiding this comment.
I can dig a little, but we don't call it from so many places and in one place we call it in the render function of a component. Probably that is why?
There was a problem hiding this comment.
Ah yeah, if you run into the blocking insert prompt, that must be it. But that should not interfere with the backup creation api call 🤔
Some users experienced a bug where they got "backup creation failed" even though the backup was successfully created.
This issue could be reproduced by forcing
sd_card_insertedto always take 1s. The logs showed that the usb processing incorrectly timed out. The processing timeout is therefore reset to about 1s more than the default.For good measure the loop checking the sd card presence is also changed to iterate more times and wait shorter times. One common case is that it runs a single iteration of the loop, and that is now much quicker.