-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Implement two-phase SD card sync with improved data safety #3934
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a significant improvement to the SD card data synchronization process by implementing a two-phase sync mechanism. This change enhances data safety by first transferring recordings from the SD card to the phone's local storage before attempting to process or upload them to the cloud. The changes are extensive, touching UI, state management, and core services. New features include the ability to cancel an ongoing SD card transfer, and the UI has been updated to display transfer speed and ETA. My review focuses on potential race conditions in the UI when displaying feedback to the user after an operation.
| // Pop back to refresh the list since WAL has moved to phone storage | ||
| Navigator.of(context).pop(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The manual Navigator.of(context).pop() call here is redundant and can cause a race condition with the SnackBar not being displayed. The UI is already designed to reactively pop the screen when the WAL's storage location changes after a successful transfer (see lines 143-151). Removing this explicit pop will make the behavior more robust and ensure the "Transfer complete" message is visible to the user.
| // Pop back since the WAL state will change | ||
| Navigator.of(context).pop(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling Navigator.of(context).pop() immediately after _showSnackBar can prevent the user from seeing the "Transfer cancelled" message, as the page's context is destroyed. To ensure the feedback is visible, you should pop the navigator after a short delay.
| // Pop back since the WAL state will change | |
| Navigator.of(context).pop(); | |
| // Pop back after a short delay to ensure the user sees the snackbar. | |
| Future.delayed(const Duration(milliseconds: 400), () => mounted ? Navigator.of(context).pop() : null); |
|
basically 1/2/ is done. but feel free to modify it especially the ui haha. |


sync audio from the device (sd card / on-device storage) to the phone first, completely, and keep it.
then sync to the STT later - don't delete anything. let users decide when to delete based on their needs.
add a fast transfer mode for omi devices that use wifi. don't forget to check whether this feature is available on the device via a feature flag.