Enable erase_chip by default #3701

haslinghuis · 2023-12-28T00:14:19Z

When migrating from previous version this settings was reported to disabled by default.

blckmn · 2023-12-28T02:17:05Z

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 -> FAIL
assigned to an approver -> PASS
approver count at least three -> FAIL

McGiverGim · 2023-12-29T07:55:01Z

src/js/tabs/firmware_flasher.js

-        if (result.erase_chip) {
+        if (result.erase_chip === undefined) {
            $('input.erase_chip').prop('checked', true);
        } else {
-            $('input.erase_chip').prop('checked', false);
+            $('input.erase_chip').prop('checked', result.erase_chip);
        }


I think is better to "overcharge" the getConfig() method with this code, adding a second "optional" parameter as "default". If configured, and undefined, then return the default. In this way we can do the same for others parameters if we want. What do you think?

Agree, its a better option, but then we have to change them all.

Why all? If the parameter is optional, we don't need to change others.

Took a shot at it. Please have a look again.

Seems good to me 😉 thanks!

src/js/tabs/firmware_flasher.js

ctzsnooze · 2024-01-04T01:57:48Z

Can I confirm the expected behaviour after this PR is as follows:

if Full Chip Erase was active when entering the Flashing tab, nothing changes.
if Full Chip Erase was not active when the Flashing tab was opened, it is changed to active, every time we enter the Flashing Tab?
if Full Chip Erase was not active when the Flashing tab was opened, and was automatically changed to active, the user can only only be reverted it to not active by clicking the switch to off?
If the user has set Full Chip Erase to off, and exits the Flashing tab, does something else in some other tab, and then returns to the Flashing tab, it will remain off?
If the user has set Full Chip Erase to off, and quits Configurator, then re-opens and goes to the Flashing tab, it will show Full Chip Erase as being on?

In the fourth possibility, that a user turns Full Chip Erase off, goes to some other tab, and comes back to the Flashing tab, I guess it is best for Full Chip Erase to stay off.

haslinghuis · 2024-01-04T12:50:03Z

In the fourth possibility, that a user turns Full Chip Erase off, goes to some other tab, and comes back to the Flashing tab, I guess it is best for Full Chip Erase to stay off.

That behavior was covered by the first commit.

nerdCopter · 2024-01-04T14:44:48Z

tested 4c5a914f:

1 = yes, remains active
2 = yes, reverts to active
3 = yes, can only manually disable
4 = no, returns to enabled
5 = yes, reverts enabled

further findings. seems flashing significantly disparate firmware version, it will still erase chip even if disabled.
i've seen this in the far past and did not comment such.
for example when disabling erase, flashing master to master works as expect, master to PR 13248 will work as expected, but some older PR like 13234 will full-chip erase.

edit: this may be a factor of some PG change affecting the entire eeprom.

haslinghuis · 2024-01-04T14:56:39Z

If the user has set Full Chip Erase to off, and exits the Flashing tab, does something else in some other tab, and then returns to the Flashing tab, it will remain off?

In the fourth possibility, that a user turns Full Chip Erase off, goes to some other tab, and comes back to the Flashing tab, I guess it is best for Full Chip Erase to stay off.

To cover point 4 - setting default enabled value in main.js instead of in firmware flasher tab would allow for session wide behavior.

nerdCopter · 2024-01-04T15:05:52Z

To cover point 4 - setting default enabled value in main.js instead of in firmware flasher tab would allow for session wide behavior.

i am okay with current behavior. unsure what others desire.

sonarcloud · 2024-01-04T22:58:00Z

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

github-actions · 2024-01-04T23:03:33Z

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!

haslinghuis added the RN: BUGFIX label Dec 28, 2023

haslinghuis added this to the 10.10.0 milestone Dec 28, 2023

haslinghuis self-assigned this Dec 28, 2023