Murdock results

✔️ PASSED

ebcb656 boards/common/ada-nrf52-bootl: refactor and enhance documentation

Artifacts

• Documentation preview

I changed the boards/common/adafruit-nrf52-bootloader/Makefile.include in a way to conditionally select the SoftDevice version. That way the board-dependant Makefile.include files don't have to define everything and can just define a different SoftDevice version if necessary (for example that will be necessary for the seeedstudio-xiao-nrf52840 because it uses SoftDevice 7 instead of SD6).

Also it is possible to drop the SoftDevice if you can't stand the idea of having a piece of closed source software in your Flash :)

Request for Comments: Should the board-dependant Makefile.include files still define the SoftDevice version, even if they use the standard?

I think this is ready for testing and reviewing now :)

To make it a bit less cluttered, I could squash it before review.
Squashed. I couldn't work with the amount of fixup!s anymore 😅

Also it is possible to drop the SoftDevice if you can't stand the idea of having a piece of closed source software in your Flash :)

Great, thanks for addressing this and making it configurable! I'm not fully decided on what the default should be though. Is it more friendly to leave the softdevice present and allow for the boards to be reused for other projects, or to not ship proprietary blobs (which are not used anyway though)? I guess both are fine with me.

Technically, we aren't shipping the blob, we're just not touching it.
The friendlier solution is certainly to keep it because it won't cause the board to be soft-locked to RIOT.

Request for Comments: Should the board-dependant Makefile.include files still define the SoftDevice version, even if they use the standard?

If there is a standard as in something that is documented in the bootloader sources as such, I'd be fine to make it the default and not define it. If not, and if it rather depends on the board, I'd propose not having a default at all and erroring out if no SD version is defined by the board.

I thought about it yesterday on my way home and even though Adafruit does not specifically set the SoftDevice version, I think they are rather fixed for a certain board. It is unlikely that there will be an update for the feather-nrf52840 for example that changes the SoftDevice version.
While having defaults won't break anything, IMO explicit is better than implicit and will make the life of someone porting a new board (similar to the feather-nrf52840), which might have a newer SD version, a lot easier.

I took the liberty to squash everything together since the last review only addressed some details in the documentation.

Other than that IMO this is ready to be merged :)

Oh, I actually just (by chance) found the following issue: When flashing a new application with the new default (expecting the SoftDevice) to a board that has previously been flashed with RIOT, it will seem to the user as if the flashing process failed as the old application is still present and started (instead of the SoftDevice).

This is pretty bad user experience, I'd say. Not sure how to prevent this... Maybe checking INFO_UF2.txt for an existing softdevice when selected, and at least warn or error in case it is not available? Maybe that could be done directly from the uf2conv.py by applying a patch?

I can also try to look into it one of these days if you don't want to invest more time here.

uf2conv has a command line option that is called --list. That returns all the mounted devices with UF2 bootloader.

RIOT/$ python3 ./dist/tools/uf2/uf2conv.py --list
/media/chris/NRF52BOOT nRF52840-pca10056-v1

With that it's pretty easy to check if the INFO_UF2.txt file has a SoftDevice. The best approach is probably to create a dedicated shell script which is executed before the programming script. I'll have to see where the best spot is for it in the make system.
If you have a good idea, I'm open for suggestions.

This is a script that ChatGPT suggested, it obviously has to be adapted still but it's a starting point.

#!/bin/bash

# Find device with UF2 bootloader
MOUNTPOINT=$(uf2conv.py --list | awk '{print $1}')

# Check if a device was found
if [ -z "$MOUNTPOINT" ]; then
    echo "No UF2 device found!"
    exit 1
fi

# Read file and check for "SoftDevice: not found"
if grep -q "SoftDevice: not found" "$MOUNTPOINT/INFO_UF2.txt"; then
    echo "SoftDevice not found!"
    exit 2
else
    echo "SoftDevice detected!"
fi

The best approach is probably to create a dedicated shell script which is executed before the programming script.

Sounds reasonable to me.

I'll have to see where the best spot is for it in the make system.
If you have a good idea, I'm open for suggestions.

I'd probably try to call the script instead of python in makefiles/tools/uf2conv.inc.mk.

I've been trying to get the re-installation of the Adafruit Bootloader somehow integrated into our build system as a pkg in dist/tools, with automatic board name mapping. Not sure if that's worth it though or if erroring out is enough for the user.

I've been trying to get the re-installation of the Adafruit Bootloader somehow integrated into our build system as a pkg in dist/tools, with automatic board name mapping. Not sure if that's worth it though or if erroring out is enough for the user.

Just for reference, here is the current WIP state: https://github.com/mguetschow/RIOT/tree/adafruit-bootloader-pkg

I've been trying to get the re-installation of the Adafruit Bootloader somehow integrated into our build system as a pkg in dist/tools, with automatic board name mapping. Not sure if that's worth it though or if erroring out is enough for the user.

For the time being, I just added an error message. Personally I'd prefer not to put the re-install of the Adafruit Bootloader into this PR as well, as it already has become quite extensive.

In the fixups, you can find a possible solution that involes a bash script that checks the INFO_UF2.txt. It could probably use some additional documentation, but this is something to work with.

Okay, I've extended the check script to check more variants that can happen and display according error messages:

This message will be shown when a version mismatch occurs (you can test that by trying to flash your Xiao as a Feather):

This message will be shown when no SoftDevice is found but there should be one. I added the URL to the documentation (the link won't work because this PR is not merged yet);

This message will be shown when there is a SoftDevice about to be overridden with the UF2_SOFTDEV=DROP command. It will NOT be shown when there is no SoftDevice present anymore and the UF2_SOFTDEV=DROP command is issued.

@mguetschow Let me know when I should squash this.

Thank you for working with me on this rather extensive PR :)