picotool icon indicating copy to clipboard operation
picotool copied to clipboard

Add error message if --force fails

Open playduck opened this issue 2 years ago • 5 comments

This PR is in response to issue #56.

Using -f or -F requires the RP2040 to have enabled USB-CDC. As this isn't immediately obvious this PR adds a short reminder if a load command with a force flag fails. This also adds a small sentence to the help load text.

Running ./picotool load binary.elf -f with no responding device now outputs:

No accessible RP2040 devices in BOOTSEL mode were found. To force a device into BOOTSEL mode, make sure the RP2040 is configured to use USB-CDC (USB stdio).

The help load text now reads (note the last sentence):

-f, --force Force a device not in BOOTSEL mode but running compatible code to reset so the command can be executed. After executing the command (unless the command itself is a 'reboot') the device will be rebooted back to application mode. Make sure the device is using USB-CDC (USB stdio)

I've also modified the the printing routine for the error message to use snprintf and strncpy to avoid potential string-length issues as is good practice.

playduck avatar Aug 31 '22 11:08 playduck

this PR adds a short reminder if a load command with a force flag fails.

It actually appears to be any command that uses the force flag, not just the load command? IMHO the behaviour you've implemented is what I'd expect, it's just that the PR description is a bit misleading :wink:

lurch avatar Aug 31 '22 12:08 lurch

Added both missing dashes, must've missed them the first time around.

It actually appears to be any command that uses the force flag

Yes, I've just been testing it with load only, but other commands behave the same:

./picotool info -f
No accessible RP2040 devices in BOOTSEL mode were found.
To force a device into BOOTSEL mode, make sure the RP2040 is configured to use USB-CDC (USB stdio).

playduck avatar Aug 31 '22 12:08 playduck

I subtracted the current string length from the maximum allowed length of the snprintf calls. This still produces the same output but is now more robust against buffer-overflows.

playduck avatar Sep 01 '22 12:09 playduck

it isn't strictly limited to USB stdio (though may be in practice as no one else implements the interface)

Perhaps "Force a device not in BOOTSEL mode but running compatible code (e.g. USB stdio)"

kilograham avatar Feb 09 '23 16:02 kilograham

You probably want to change this to be based against develop instead of master? And whilst doing that, be aware of the change introduced in #72 :wink:

lurch avatar Feb 19 '23 23:02 lurch