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.

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

Make virtual mode an option using msp backup and restore #2748

Merged
merged 2 commits into from Jan 13, 2022
Merged

Make virtual mode an option using msp backup and restore #2748

merged 2 commits into from Jan 13, 2022

Conversation

haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Jan 10, 2022 •
edited

After some confusion in #2742 we decided to:

  • Remove virtual connection from PortHandler. [1]
  • Make virtual connection an option as most people don't use this feature. [1]
  • Restore backup and restore if option is enabled to be able to backup a real target and restore on a virtual one.
  • Remove restore button in serial mode from firmware >= 4.3 if virtual option enabled.
  • Move showAllSerialDevices to PortHandler for optimization.

[1] please re-enable Virtual Mode in Options tab if needed.

image

@haslinghuis haslinghuis added this to the 10.8.0 milestone Jan 10, 2022
@haslinghuis haslinghuis added this to For discussion in Finalizing Firmware 4.3 Release via automation Jan 10, 2022
@haslinghuis haslinghuis moved this from For discussion to Configurator in Finalizing Firmware 4.3 Release Jan 10, 2022
@haslinghuis haslinghuis force-pushed the restore_backup_virtual branch 2 times, most recently from 83fb4cb to 534cad5 Compare January 10, 2022 03:35
@haslinghuis haslinghuis self-assigned this Jan 10, 2022
@haslinghuis haslinghuis force-pushed the restore_backup_virtual branch 2 times, most recently from 6f2d941 to 856348d Compare January 10, 2022 04:15
@asizon
Copy link
Member

asizon commented Jan 10, 2022

Wow thankyou for the changes @haslinghuis
Maybe we should put virtual mode in the button?some like "Backup(Only VirtualMode)" or similar.

@asizon asizon added the Tested label Jan 10, 2022
McGiverGim
McGiverGim reviewed Jan 10, 2022
View reviewed changes
locales/en/messages.json Outdated
@@ -109,6 +109,10 @@
"message": "Show all serial devices (for manufacturers or development)",
"description": "Do not filter serial devices using VID/PID values (for manufacturers or development)"
},
"showVirtualMode": {
"message": "Enable virtual connection mode",
"description": "Hide virtual connection mode by default"
Copy link
Member

Choose a reason for hiding this comment

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

This description is only for the translators. It must "describe" what are they seeing to help them translate them.

Edit: something like Text for the option to enable or disable the virtual FC

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed!

@blckmn
Copy link
Member

blckmn commented Jan 11, 2022

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 -> PASS
  • 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

@limonspb
Copy link
Member

limonspb commented Jan 12, 2022 •
edited

Speaking of Backup and Restore buttons on Setup Tab:

  1. Keeping these buttons as is for 4.2 means we will keep misleading 4.2 users.
    They are broken not just in 4.3 but also in 4.2 and who knows about 4.1.
    For example in 4.2 they are not restoring OSD, Power tab, Vtx table, filters, advanced PID parameters and who knows what else.

  2. in 4.3 they are even more broken.

  3. HOWEVER i can see why some testers still like to use these buttons.

Considering 1.2.3 we need to keep these buttons for all FW versions, but we need make sure only testers and people that know what they are doing are hitting these buttons.

I am proposing to rename these buttons to "Save Json, Load Json" and moving them into a separate "Testing" panel, something like that (and make them always visible):
image

that way the current functionality (broken or not) is still there for testers and @asizon, but it is not confusing regular users anymore.

@sonarcloud
Copy link

sonarcloud bot commented Jan 12, 2022

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

@ctzsnooze ctzsnooze self-requested a review January 13, 2022 01:02
asizon
asizon approved these changes Jan 13, 2022
View reviewed changes
Copy link
Member

@asizon asizon left a comment

Choose a reason for hiding this comment

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

Lgtm! Thankyou @haslinghuis

Finalizing Firmware 4.3 Release automation moved this from Configurator to Firmware Jan 13, 2022
@asizon asizon merged commit dc33fb9 into betaflight:master Jan 13, 2022
Finalizing Firmware 4.3 Release automation moved this from Firmware to Finished (Merged) Jan 13, 2022
@haslinghuis haslinghuis deleted the restore_backup_virtual branch January 13, 2022 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@limonspb limonspb limonspb approved these changes

@ctzsnooze ctzsnooze ctzsnooze approved these changes

@asizon asizon asizon approved these changes

@McGiverGim McGiverGim Awaiting requested review from McGiverGim

Assignees

@haslinghuis haslinghuis

Labels
RN: BUGFIX Tested
Projects
No open projects
Finalizing Firmware 4.3 Release
Finished (Merged)
Milestone
10.8.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@haslinghuis @asizon @blckmn @limonspb @McGiverGim @ctzsnooze