tools/bootloader: add force-erase option and add bootloader version (Sponsored by CubePilot) #22777

julianoes · 2024-02-19T23:05:21Z

This PR does two things:

Add force-erase option

If the STM32H7 fails to program or erase a full chunk of 256 bytes, the ECC check will trigger a busfault when trying to read from it.

To speed up erasing and optimize wear, we read before erasing to check if it actually needs erasing. That's when a busfault happens and the erase time outs.

The workaround is to add an option to do a full erase without check.

Credit goes to:
ArduPilot/ardupilot#22090

And the protocol option added to the bootloader is the same as for ArduPilot, so compatible.

Add bootloader version

This PR also adds a new bootloader version essentially a string that we can set in the format of: PX4BLv1.15.0g12345678 constrained to max 32 chars. This allows to check if a bootloader needs updating while not breaking any tooling by bumping the bootloader protocol revision which has been stabilized at 5 for a long time by now.

The output of the px_uploader.py now looks as follows:

uploading px4
Waiting for bootloader...

Found board id: 140,0 bootloader protocol revision 5 on /dev/serial/by-id/usb-CubePilot_PX4_BL_CubePilot_CubeOrange_0-if00
Loaded firmware for board id: 140,0 size: 1902144 bytes (96.75%) 

Bootloader version: PX4BLv1.15.0g3bbeb0d87d
Sn: 003100433230510b39343831
Chip: 20036450
Family: STM32H7[4|5]x
Revision: V
Flash: 1966080 bytes
Windowed mode: False

Erase  : [====================] 100.0%
Program: [======              ] 33.9%

davids5

@julianoes Nice! The wording on the option felt wrong to me. See if what I added works for you.

Tools/px_uploader.py

julianoes · 2024-02-22T04:33:13Z

Fixed, thanks @davids5!

davids5

Thinking about this a bit more....Since this is s protocol change should we bump the rev and add it to https://github.com/PX4/PX4-Bootloader?

julianoes · 2024-02-22T18:07:07Z

@davids5 we should but we haven't in the past, and it looks like ArduPilot has many more features and is also still on the same revision.

julianoes · 2024-02-27T04:12:24Z

@davids5 it doesn't need to go into the Bootloader repo, because it only applies for H7, I think.

julianoes · 2024-02-27T06:51:52Z

@davids5 I had messed up. Here is a fixup commit: ac5b06f It would be great if you could double check that line again. Thanks!

Needs rebasing/squashing before merging.

davids5

See inlines

platforms/nuttx/src/bootloader/nxp/imxrt_common/main.c

julianoes · 2024-02-27T07:26:30Z

Pushed another fixup @davids5.

platforms/nuttx/src/bootloader/nxp/imxrt_common/main.c

davids5

I think it would be good to use
up_progmem_ispageerased as the name, see comment

platforms/nuttx/src/bootloader/nxp/imxrt_common/main.c

julianoes · 2024-02-28T02:58:44Z

@davids5 ok, I've accepted your suggestions. I'll run CI, then rebase, squash and force push.

davids5

On the issue of the revision - The uploader side can blindly send the command and it can fail silently.

As far as putting this in the the bootloader repo: It would be a nice to have but will not fit on all the SoC.

davids5

The rest all looks good. This is just a nit. I will leave it up to you, if you want to push again :)

platforms/nuttx/src/bootloader/common/bl.c

julianoes · 2024-02-29T21:54:23Z

@davids5 on second thought, I think you're right that it would be worthwhile to bump the version number.

davids5

Ok Looks good!

julianoes · 2024-03-04T17:05:28Z

Thanks @davids5. I'll properly squash the commits and then merge it.

julianoes · 2024-03-04T17:38:12Z

@dagar what's an easy way to update all bootloader images? Do I have to do it one by one?

davids5 · 2024-03-14T10:40:53Z

@julianoes There is a target iirc. something like update_bootloaders

julianoes · 2024-03-18T02:16:47Z

@davids5 thanks, I'll try that. However, I wonder whether we should really bump the protocol version. The problem is that QGC refused to flash bootloader rev 6, and so does the uploader script, and that's even though they would just work fine. Instead of a "major" change it's more like a hidden extra feature. On the other hand, it's hard to know whether a flashed bootloader supports the force-erase option. You basically don't know until you try.

davids5 · 2024-03-18T13:07:05Z

@davids5 thanks, I'll try that. However, I wonder whether we should really bump the protocol version. The problem is that QGC refused to flash bootloader rev 6, and so does the uploader script, and that's even though they would just work fine. Instead of a "major" change it's more like a hidden extra feature. On the other hand, it's hard to know whether a flashed bootloader supports the force-erase option. You basically don't know until you try.

So it would force an update for QGC and the uploader on the users. How bad is that?

julianoes · 2024-03-18T15:06:26Z

How bad is that?

Not the end of the world but still unexpected.

julianoes · 2024-05-10T01:11:39Z

@davids5 this has evolved a bit. @bugobliterator and I came up with a bootloader version additional to the bootloader protocol revision. For more info, see the PR description and the individual commits. I also took the liberty to clean up the Python script a bit.

I'd appreciate your review @davids5.

FYI @dagar

TODO: re-generate all bootloaders from here.

davids5 · 2024-05-10T16:30:16Z

@davids5 this has evolved a bit. @bugobliterator and I came up with a bootloader version additional to the bootloader protocol revision. For more info, see the PR description and the individual commits. I also took the liberty to clean up the Python script a bit.

I'd appreciate your review @davids5.

FYI @dagar

TODO: re-generate all bootloaders from here.

@julianoes - can we have a call on this? I am in US EST so maybe 4 AM my time next week or 03:00 PM EST. Let me know

julianoes · 2024-05-17T03:16:27Z

@davids5 I have the feeling the GET_SUPPORTED_COMMANDS API that you suggested is - unfortunately - not really worth the trouble.

It seems worthwhile because it would solve the problem where we don't know if force-erase is supported or not, basically if GET_SUPPORTED_COMMANDS is not supported, force-erase won't be either. However, this doesn't hold for the ArduPilot bootloader, so we're sort of back at square one.

I agree that it would only be worthwhile for the next bootloader addition down the road, but it doesn't seem quite worth it, because you can easily just "try" every command, except erase which take a long time to time out.

davids5

See inline.

davids5 · 2024-05-17T05:28:21Z

Tools/px_uploader.py

                return

-        raise RuntimeError("timed out waiting for erase")
+        if self.force_erase:
+            raise RuntimeError("timed out waiting for erase, force erase is likely not supported by bootloader!")


Wouldn't the the force erase bet met with an immediate command error? In that case should you attempt to use an erase or suggest a bootloader upgrade?

Yes, that would be nice. But a unknown byte seems to just be ignored. And the erase command is not acked until it is erased, so you have to wait a long time. Not optimal I know...

geez That seems wrong, bit it is what the code does.

If the STM32H7 fails to program or erase a full chunk of 256 bytes, the ECC check will trigger a busfault when trying to read from it. To speed up erasing and optimize wear, we read before erasing to check if it actually needs erasing. That's when a busfault happens and the erase time outs. The workaround is to add an option to do a full erase without check. Credit goes to: ArduPilot/ardupilot#22090 And the protocol option added to the bootloader is the same as for ArduPilot, so compatible. Signed-off-by: Julian Oes <julian@oes.ch>

This adds a new protocol extension which allows to get the bootloader version. The bootloader version is different from the bootloader protocol revision which has stabilized at 5 and is not easy to update unless a bootloader is actually breaking the protocol. The reason being that both the Python script as well as the uploader used in QGC will not attempt to load firmware if they don't know the bootloader version, so it could basically be considered a "breaking" protocol revision. Signed-off-by: Julian Oes <julian@oes.ch>

Includes: - Remove some of the outdated Python2 checks and compatibility. - Try not catch all exceptions but only the expected ones. Otherwise, this makes it really hard to debug if anything unexpected actually goes wrong. - Make use of fstrings. - Make output slightly prettier. Signed-off-by: Julian Oes <julian@oes.ch>

Just so we don't conflict on these commands in the future.

Signed-off-by: Julian Oes <julian@oes.ch>

julianoes · 2024-05-22T05:15:19Z

Final rebase, bootloader rebuild and push. As soon as CI is happy I'll merge.

julianoes · 2024-05-22T06:18:46Z

CI results are the same as main. Merging.

PetervdPerk-NXP · 2024-05-23T11:54:31Z

px4_fmu-v6xrt: Build regression on main since this PR.

/home/peter/src/Firmware/platforms/nuttx/src/bootloader/nxp/imxrt_common/main.c:399:1: error: return type defaults to 'int' [-Werror=implicit-int]
  399 | up_progmem_ispageerased(unsigned sector)
      | ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated due to -Wfatal-errors.
cc1: all warnings being treated as errors

Commit 4a13e49 was succesful on px4_fmu-v6xrt before this PR.

davids5 · 2024-05-23T12:34:28Z

px4_fmu-v6xrt: Build regression on main since this PR.

/home/peter/src/Firmware/platforms/nuttx/src/bootloader/nxp/imxrt_common/main.c:399:1: error: return type defaults to 'int' [-Werror=implicit-int]
  399 | up_progmem_ispageerased(unsigned sector)
      | ^~~~~~~~~~~~~~~~~~~~~~~
compilation terminated due to -Wfatal-errors.
cc1: all warnings being treated as errors

Commit 4a13e49 was succesful on px4_fmu-v6xrt before this PR.

Fixed in #23174

julianoes requested a review from davids5 February 19, 2024 23:05

davids5 requested changes Feb 20, 2024

View reviewed changes

Tools/px_uploader.py Outdated Show resolved Hide resolved

davids5 requested changes Feb 22, 2024

View reviewed changes

davids5 requested changes Feb 27, 2024

View reviewed changes

platforms/nuttx/src/bootloader/nxp/imxrt_common/main.c Show resolved Hide resolved

platforms/nuttx/src/bootloader/nxp/imxrt_common/main.c Outdated Show resolved Hide resolved

davids5 requested changes Feb 27, 2024

View reviewed changes

platforms/nuttx/src/bootloader/nxp/imxrt_common/main.c Outdated Show resolved Hide resolved

platforms/nuttx/src/bootloader/nxp/imxrt_common/main.c Outdated Show resolved Hide resolved

platforms/nuttx/src/bootloader/nxp/imxrt_common/main.c Outdated Show resolved Hide resolved

davids5 requested changes Feb 27, 2024

View reviewed changes

platforms/nuttx/src/bootloader/nxp/imxrt_common/main.c Outdated Show resolved Hide resolved

davids5 reviewed Feb 28, 2024

View reviewed changes

platforms/nuttx/src/bootloader/common/bl.c Show resolved Hide resolved

julianoes changed the title ~~tools/bootloader: add force-erase option~~ [DON'T MERGE yet] tools/bootloader: add force-erase option Feb 29, 2024

julianoes force-pushed the pr-bootloader-full-erase branch from 77fab46 to 42ef9ce Compare March 3, 2024 17:05

davids5 previously approved these changes Mar 4, 2024

View reviewed changes

julianoes force-pushed the pr-bootloader-full-erase branch from 42ef9ce to 7f3f809 Compare March 4, 2024 17:25

julianoes changed the title ~~[DON'T MERGE yet] tools/bootloader: add force-erase option~~ tools/bootloader: add force-erase option Mar 4, 2024

julianoes changed the title ~~tools/bootloader: add force-erase option~~ [Sponsored by CubePilot] tools/bootloader: add force-erase option and add bootloader version May 10, 2024

julianoes dismissed davids5’s stale review via 13f2ca2 May 10, 2024 01:09

julianoes force-pushed the pr-bootloader-full-erase branch from 7f3f809 to 13f2ca2 Compare May 10, 2024 01:09

julianoes force-pushed the pr-bootloader-full-erase branch from a84c75f to b847765 Compare May 17, 2024 03:32

davids5 self-requested a review May 17, 2024 05:21

davids5 requested changes May 17, 2024

View reviewed changes

julianoes force-pushed the pr-bootloader-full-erase branch from 38e2dda to 7678d21 Compare May 20, 2024 03:09

davids5 self-requested a review May 21, 2024 13:59

davids5 previously approved these changes May 21, 2024

View reviewed changes

julianoes changed the title ~~[Sponsored by CubePilot] tools/bootloader: add force-erase option and add bootloader version~~ tools/bootloader: add force-erase option and add bootloader version (Sponsored by CubePilot) May 22, 2024

julianoes added 7 commits May 22, 2024 17:05

bootloader: track ArduPilot protocol

b787d8a

Just so we don't conflict on these commands in the future.

bootloader: remove unused/duplicate defines

f67b19d

px_uploader: catch serial exception correctly

1e89680

Signed-off-by: Julian Oes <julian@oes.ch>

boards: update all bootloaders

d6c9f4a

julianoes dismissed davids5’s stale review via d6c9f4a May 22, 2024 05:15

julianoes force-pushed the pr-bootloader-full-erase branch from 7678d21 to d6c9f4a Compare May 22, 2024 05:15

julianoes merged commit a9962dc into main May 22, 2024
87 of 93 checks passed

julianoes deleted the pr-bootloader-full-erase branch May 22, 2024 06:18

This was referenced May 30, 2024

[BACKPORT release/1.15] tools/bootloader: add force-erase option and add bootloader version #23210

Merged

Fix bootloaders_update target and update all bootloaders #23234

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tools/bootloader: add force-erase option and add bootloader version (Sponsored by CubePilot) #22777

tools/bootloader: add force-erase option and add bootloader version (Sponsored by CubePilot) #22777

julianoes commented Feb 19, 2024 •

edited

davids5 left a comment

julianoes commented Feb 22, 2024

davids5 left a comment

julianoes commented Feb 22, 2024

julianoes commented Feb 27, 2024

julianoes commented Feb 27, 2024 •

edited

davids5 left a comment

julianoes commented Feb 27, 2024

davids5 left a comment •

edited

julianoes commented Feb 28, 2024

davids5 left a comment

davids5 left a comment

julianoes commented Feb 29, 2024

davids5 left a comment

julianoes commented Mar 4, 2024

julianoes commented Mar 4, 2024

davids5 commented Mar 14, 2024

julianoes commented Mar 18, 2024

davids5 commented Mar 18, 2024

julianoes commented Mar 18, 2024

julianoes commented May 10, 2024 •

edited

davids5 commented May 10, 2024

julianoes commented May 17, 2024

davids5 left a comment

davids5 May 17, 2024

julianoes May 18, 2024

davids5 May 21, 2024

julianoes May 21, 2024

julianoes commented May 22, 2024

julianoes commented May 22, 2024

PetervdPerk-NXP commented May 23, 2024 •

edited

davids5 commented May 23, 2024

tools/bootloader: add force-erase option and add bootloader version (Sponsored by CubePilot) #22777

tools/bootloader: add force-erase option and add bootloader version (Sponsored by CubePilot) #22777

Conversation

julianoes commented Feb 19, 2024 • edited

Add force-erase option

Add bootloader version

davids5 left a comment

Choose a reason for hiding this comment

julianoes commented Feb 22, 2024

davids5 left a comment

Choose a reason for hiding this comment

julianoes commented Feb 22, 2024

julianoes commented Feb 27, 2024

julianoes commented Feb 27, 2024 • edited

davids5 left a comment

Choose a reason for hiding this comment

julianoes commented Feb 27, 2024

davids5 left a comment • edited

Choose a reason for hiding this comment

julianoes commented Feb 28, 2024

davids5 left a comment

Choose a reason for hiding this comment

davids5 left a comment

Choose a reason for hiding this comment

julianoes commented Feb 29, 2024

davids5 left a comment

Choose a reason for hiding this comment

julianoes commented Mar 4, 2024

julianoes commented Mar 4, 2024

davids5 commented Mar 14, 2024

julianoes commented Mar 18, 2024

davids5 commented Mar 18, 2024

julianoes commented Mar 18, 2024

julianoes commented May 10, 2024 • edited

davids5 commented May 10, 2024

julianoes commented May 17, 2024

davids5 left a comment

Choose a reason for hiding this comment

davids5 May 17, 2024

Choose a reason for hiding this comment

julianoes May 18, 2024

Choose a reason for hiding this comment

davids5 May 21, 2024

Choose a reason for hiding this comment

julianoes May 21, 2024

Choose a reason for hiding this comment

julianoes commented May 22, 2024

julianoes commented May 22, 2024

PetervdPerk-NXP commented May 23, 2024 • edited

davids5 commented May 23, 2024

julianoes commented Feb 19, 2024 •

edited

julianoes commented Feb 27, 2024 •

edited

davids5 left a comment •

edited

julianoes commented May 10, 2024 •

edited

PetervdPerk-NXP commented May 23, 2024 •

edited