betaflight / betaflight

Open Source Flight Controller Firmware
GNU General Public License v3.0
8k stars 2.86k forks source link

Remove functionality to enter flash bootloader by sending 'F' on a UART configued for MSP. #13572

Closed hydra closed 3 weeks ago

hydra commented 3 weeks ago

The proper way to reboot to the boot loader is by sending an MSP_REBOOT command.

It was added here: https://github.com/betaflight/betaflight/commit/5cf42f40b6dddd64d59738b3420ad045bf926711#diff-00049baa371593fe10572252c0aff1498ed119bc38c4bedf404143d9a00ac0b2R435-R437

I came across this when developing an ELRS VTX addon board for the H7NEO/H7EVO and had 'MSP' enabled on UART1, when the device is powered on from cold boot there are startup messages emitted by the ROM of the ELRS MCU over it's serial port and one of the characters is an 'F' which causes the FC to reboot into bootloader mode.

Since this only happened on a cold boot, it was very tricky to find the cause as normally debugging methods could not be employed.

Watch the video

Additionally, entering the CLI using a single '#' character is probably also bad and could similarly be triggered accidentally.

This PR deletes this unwanted functionality and also serves to determine if the functionality is really needed or not, by way of responses to it.

As far as I know, being the only person and manufacturer of boards using a flash bootloader, we don't need this functionality.

github-actions[bot] commented 3 weeks ago

Do you want to test this code? You can flash it directly from Betaflight Configurator:

WARNING: It may be unstable. Use only for testing!

hydra commented 3 weeks ago

Might solve other issues booting intermittent in DFU like found before.

Probably, especially in the case of long serial cables with noise/corruption. There is code guarding against going into bootloader or cli when armed, so this only happens when disarmed.

The old CLI '#' code, and 'R' before it are very, very old bits of legacy code, as seen here: https://github.com/betaflight/betaflight/commit/19ca85963bbc7bcac6520e54399f249d692c0871#diff-1e79d5ff1cf01966894b31a5635f22fb91fea7b6a991373064f2837ac97e147fR361

The 'F' functionality that is PR deletes should probably have never been added.