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

Make backup on flash an option #4360

Merged
merged 12 commits into from
Mar 9, 2025
Merged

Conversation

haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Feb 25, 2025

  • adds option for backup during flashing (disabled, enabled, ask)
  • enabled is default
  • add error handling in filesystem API if there is no file handler available
  • add error handling for backup process
  • increase timeout to enter cli for backup command to reduce corruption of resulting config
  • remove some unused messages
  • fix parameter for i18n message
  • refactor to make sonar happy

image

@haslinghuis haslinghuis added this to the 11.0 milestone Feb 25, 2025
@haslinghuis haslinghuis self-assigned this Feb 25, 2025
Copy link

netlify bot commented Feb 25, 2025

Deploy Preview for origin-betaflight-app ready!

Name Link
🔨 Latest commit 57e3920
🔍 Latest deploy log https://app.netlify.com/sites/origin-betaflight-app/deploys/67cc80ef491ef10008d7d05e
😎 Deploy Preview https://deploy-preview-4360.dev.app.betaflight.com
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@haslinghuis haslinghuis force-pushed the backup branch 2 times, most recently from 4ce022e to c3120c2 Compare February 25, 2025 23:08
@nerdCopter

This comment was marked as outdated.

@haslinghuis
Copy link
Member Author

Sorry need to move default to main.js (as now it adds default in option).

@nerdCopter

This comment was marked as outdated.

@nerdCopter
Copy link
Member

nerdCopter commented Feb 26, 2025

i don't know that i would keep this option ON. (but also like master's current option)
what are some alternatives?

  • backup button on bottom of screen before Exit DFU?
  • new left-menu tab Backup/Restore?

@haslinghuis
Copy link
Member Author

haslinghuis commented Feb 26, 2025

Perhaps:

  • new users rejecting the dialog - not having a backup when flashing new firmware ==> force backup (and keep option ?)
  • advanced users want to backup in certain situations ==> keep the dialog (and option so dialog can be disabled)

Not for now:
Thinking about a window for complete Backup - Flash - Restore process (need to keep track of flash process and exceptions)

@nerdCopter
Copy link
Member

nerdCopter commented Feb 26, 2025

  • meanwhile, is it possible to have master's backup/ignore dialog every flash?
  • maybe the option to turn this dialog off
  • maybe this is best alternative, but have the dialog more "alert"

@haslinghuis haslinghuis force-pushed the backup branch 5 times, most recently from 220c09d to 8220ca9 Compare February 26, 2025 18:37
@haslinghuis
Copy link
Member Author

@nerdCopter think have addressed your review

@haslinghuis haslinghuis force-pushed the backup branch 3 times, most recently from ccb7ce9 to 4e4f466 Compare February 26, 2025 18:59
@nerdCopter
Copy link
Member

nerdCopter commented Feb 26, 2025

please set ask before backup as default

edit: i think enabled by default, pardon my change of mind, it was when the option-save was broken (displaying disabled).

@nerdCopter

This comment was marked as outdated.

@haslinghuis
Copy link
Member Author

@nerdCopter should overwrite once set. Saves for me (changing or refreshing tab in browser)

@nerdCopter

This comment was marked as outdated.

@haslinghuis
Copy link
Member Author

Hmmm, works locally but not in preview 🤯

@nerdCopter

This comment was marked as outdated.

@haslinghuis
Copy link
Member Author

Seems to work now after we accept this dialog

image

@nerdCopter
Copy link
Member

nerdCopter commented Feb 26, 2025

dd00823e
seems to work as expected. let's ask on discord about default = enable vs ask.

edit: in retrospect, i think enabled by default

@haslinghuis
Copy link
Member Author

Rebased on master

@haslinghuis haslinghuis requested a review from limonspb February 27, 2025 21:07
@TehllamaFPV
Copy link

Testing on my end so far so good. I'll try and put it through the wringer tonight.
I did have some dumb questions about default file locations (i.e. can I set these to exist in a sub-folder), but I'll test those out first.
I need to figure out if having a standalone button on the firmware flasher page (for FCs not in DFU mode) would make sense for 'make backup file in default location, put FC into DFU mode, and select the target from FC as the target, user may now proceed with selection BF version and options [and if doing some javascript to fish for particular things like RX serial providers on separate lists to auto-enable certain options like SBUS/DSMX], probably a separate PR since this is a solid answer to 90% of the problem space.

@haslinghuis haslinghuis requested a review from blckmn March 4, 2025 19:33
Copy link
Member

@VitroidFPV VitroidFPV left a comment

Choose a reason for hiding this comment

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

All options work as they should.

The only thing I am a little confused about is that when using Backup Enabled, and the file picker dialog is closed, the Configurator won't open the selection for the DFU device (this seems appropriate), but the board stays in DFU mode. It'd be nice (if possible) to automatically exit DFU after that

@nerdCopter
Copy link
Member

@VitroidFPV , just in case: if you accidentally click out of chrome, the DFU dialog disappears. refresh seems to be the only answer.

i re-tested your observation by resetting my permissions. refreshing, backup-enabled. canceled the save dialog. STM32 DFU popped up for me. Debain Linux, Chrome.

Copy link

sonarqubecloud bot commented Mar 8, 2025

@haslinghuis haslinghuis merged commit 544dddd into betaflight:master Mar 9, 2025
9 checks passed
@haslinghuis haslinghuis deleted the backup branch March 9, 2025 00:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants