Skip to content

Backup config on flash #3459

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Jun 13, 2023
Merged

Backup config on flash #3459

merged 4 commits into from
Jun 13, 2023

Conversation

@haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented May 22, 2023

Saves the configuration on the flight controller before flashing.

@haslinghuis haslinghuis added this to the 10.10.0 milestone May 22, 2023
@haslinghuis haslinghuis self-assigned this May 22, 2023
@haslinghuis haslinghuis force-pushed the backup-on-flash branch 2 times, most recently from 46c5c86 to ac1b33e Compare May 22, 2023 02:47
@blckmn
Copy link
Member

blckmn commented May 22, 2023 •
edited
Loading

AUTOMERGE: (FAIL)

  • github identifies PR as mergeable -> FAIL
  • assigned to a milestone -> PASS
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> FAIL
  • Don't merge label NOT found -> PASS
  • at least one RN: label found -> PASS
  • Tested label found -> PASS
  • assigned to an approver -> PASS
  • approver count at least three -> FAIL

@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@haslinghuis haslinghuis force-pushed the backup-on-flash branch 2 times, most recently from 50fe609 to ac1b33e Compare May 23, 2023 22:44
@github-actions

This comment was marked as outdated.

Sign in to view
@haslinghuis
Copy link
Member Author

haslinghuis commented May 31, 2023

Error building NW apps: Error: certificate has expired

@github-actions

This comment was marked as outdated.

Sign in to view
@github-actions

This comment was marked as outdated.

Sign in to view
@github-actions

This comment was marked as outdated.

Sign in to view
@github-actions

This comment was marked as outdated.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@McGiverGim
Copy link
Member

McGiverGim commented Jun 1, 2023

Error building NW apps: Error: certificate has expired

Some error with some certificate in the middle, I suppose on the nw.js server to download the nw zip. But it seems fixed now.

@limonspb
Copy link
Member

limonspb commented Jun 5, 2023

I think the filename should also have date/time in the name? So we don't overwrite the old ones/previous drone flashes?

@haslinghuis haslinghuis force-pushed the backup-on-flash branch 2 times, most recently from 1aa09fb to e759ef3 Compare June 6, 2023 10:41
@haslinghuis
Copy link
Member Author

haslinghuis commented Jun 6, 2023

@TheIsotopes Please test this PR (temporary disables the tile as the content of the tile needs some more love)

@github-actions

This comment has been minimized.

Sign in to view
@TheIsotopes
Copy link
Contributor

TheIsotopes commented Jun 6, 2023

@haslinghuis Seems to be working so far.
status, diff all and dump all are saved to one file.

@nerdCopter
Copy link
Member

nerdCopter commented Jun 6, 2023

this is fantastic!
please use the existing naming convention, just adding _backup.
e.g. BTFL_cli_backup_20230427_165129_SKYSTARSF405AIO.txt

@haslinghuis
Copy link
Member Author

haslinghuis commented Jun 10, 2023 •
edited
Loading

Agree. We'd rather have a dump all only imo. Also status is not important here.

Updated to dump all.

EDIT: after discussion on Discord we revert to diff all defaults so it can be used for easy recovery.

@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@nerdCopter
Copy link
Member

nerdCopter commented Jun 12, 2023

  • tested 3 consecutive flashes successfully (without leaving tab)
  • long wait after file-save ☹️
  • only first flash presents the warning dialog.

@github-actions

This comment has been minimized.

Sign in to view
@haslinghuis
Copy link
Member Author

haslinghuis commented Jun 12, 2023

Thanks for testing @nerdCopter

  • long wait after file-save ☹️

This should now be optimized with updated timer and added interval.

  • only first flash presents the warning dialog.

It's a daily session timer 😏

@nerdCopter
Copy link
Member

nerdCopter commented Jun 12, 2023

thank you. i dont mind the daily-timer. the long-wait is somewhat unnerving. i realize it was added for OSX. Others should not have to suffer 😆

@haslinghuis
Copy link
Member Author

haslinghuis commented Jun 13, 2023 •
edited
Loading

Initial timeout of 500ms is definite not enough (about 1500ms should be considered save)

image

@haslinghuis haslinghuis force-pushed the backup-on-flash branch 2 times, most recently from 59b3a5c to ff35fbf Compare June 13, 2023 02:39
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
nerdCopter
nerdCopter approved these changes Jun 13, 2023
View reviewed changes
Copy link
Member

@nerdCopter nerdCopter left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • approving, acceptable waits for benefit of automated backups.
  • tested good multiple flashes; two FC's
  • Tested on Linux Debian 11

@haslinghuis
Copy link
Member Author

haslinghuis commented Jun 13, 2023

Rebased on master

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 13, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@github-actions
Copy link
Contributor

github-actions bot commented Jun 13, 2023

Do you want to test this code? Here you have an automated build:
Betaflight-Configurator-Android
Betaflight-Configurator-Linux
Betaflight-Configurator-macOS
Betaflight-Configurator-Windows
WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!

@blckmn blckmn merged commit 5621658 into betaflight:master Jun 13, 2023
@haslinghuis haslinghuis deleted the backup-on-flash branch June 13, 2023 23:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@KarateBrot KarateBrot KarateBrot left review comments

@blckmn blckmn blckmn approved these changes

@nerdCopter nerdCopter nerdCopter approved these changes

@McGiverGim McGiverGim Awaiting requested review from McGiverGim

+1 more reviewer

@TheIsotopes TheIsotopes TheIsotopes approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@haslinghuis haslinghuis

Labels

RN: IMPROVEMENT Tested

Projects

Finalizing Firmware 4.5 Release
Status: COMPLETED

Milestone

10.10.0

Development

Successfully merging this pull request may close these issues.

7 participants

@haslinghuis @blckmn @McGiverGim @limonspb @TheIsotopes @nerdCopter @KarateBrot