Implement two-phase SD card sync with improved data safety #3934
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
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.
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. |
|
|
|
ScreenRecording_12-29-2025.20-50-59_1.MP4 |
|
beastoin
left a comment
beastoin
left a comment
There was a problem hiding this comment.
lgtm sir @mdmohsin7
|
-- 2/ Can we automatically create the SSID and password without asking the user? 3/ I’m not sure if the current sync mode is Wi-Fi or BLE. It might be better to let users choose whether they want to use BLE or Wi-Fi for the next sync, or at least allow them to switch. 4/ Can we display the speed after 5 seconds of syncing and recalculate it every 5 seconds? I think it would improve the UX since waiting too long for the first bytes might lead users to think our sync is slow. 5/ Is BLE sync now 17KB/s? I remember it being 30KB/s last time. Just double-checking. 6/ After a while, the app shows "Device disconnected," and the device's LED is still blue, but I cannot find it using either Omi or NRF Connect. I turned the device off by pressing the button, but now I cannot turn it on again. So, red flag. 7/ I also cannot find a way to update my SSID/password for my hotspot. If allowing users to enter it manually is the only way, we need to let users edit it. It would be great if we could run some tests to let users know that their hotspot is good to go. 8/ On the firmware side, please also handle cases where the hotspot credentials are incorrect to protect the device. (updates: add 6, 7, 8) app: testflight 1.0.517 (553) |
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.
#3847