Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: piku deploy fails on MacroPad RP2040 #14

Open
tammymakesthings opened this issue Mar 21, 2022 · 3 comments
Open

Bug: piku deploy fails on MacroPad RP2040 #14

tammymakesthings opened this issue Mar 21, 2022 · 3 comments
Assignees

Comments

@tammymakesthings
Copy link
Contributor

tammymakesthings commented Mar 21, 2022

The piku deploy command fails on MacroPad RP2040 boards. Tracing this out, I see that this is because piku/commands/deploy.py has a function, has_correct_size(path) that checks if the mounted volume is larger than 3 MB, and the MacroPad RP2040 has 8MB of flash.

I was looking at how to fix this (and coded a board-specific workaround for the RP2040 case), but I brought it up in the CircuitPython weekly meeting this week to see if there’s a better way to do this. I think I’m going to submit a PR to add some helpers to the Adafruit_Board_Toolkit library for probing the board architecture and other stuff you can find out from parsing boot_out.txt. But @danh and @tannewt pointed out in the meeting that some of the Broadcom-based CPY boards (and maybe an increasing percentage of other boards in the future) use an SD card for their flash storage, so the size of the flash could be arbitrarily large on those boards.

So, I guess the open question before I continue with my fix is: What is the error case the has_correct_size() check is trying to cover? If we’re merely trying to make sure the device we’ve identified at path is a CircuitPython board, maybe it’s sufficient to check if there’s a boot_out.txt file at that path? I’m happy to to submit a PR to fix the issue, but I’d like to get @mraleson ’s input on what the problem we’re solving for is before I go too far down the solutioning rabbit hole.

@mraleson
Copy link
Owner

Thank you for noting this. Yes my intention there was basically a safety feature so that no one accidentally wipes out their flash drive (or worse a hard drive).

Maybe for the time being we could increase that to something that covers all the non SD card boards? In the long run I think it would be great to include some sophisticated check for the boot loader from Adafruit_Board_Toolkit and maybe a warning and confirmation if the board was over a certain size? I think that checking the existence of the boot_out.txt file is not enough assurance in and of itself. What are your thoughts?

If we did check for the bootloader / bootloader version it would also be nice to be able to inform someone that micropython wasn't installed on the device yet and direct them.

@tammymakesthings
Copy link
Contributor Author

As a developer, my preference is to avoid “do something for now and fix it later” solutions if I can help it. It seems those always just end up deferring the painful solution until later. :-)

What if we did something like this:

  • Check that the target path contains a boot_out.txt file, and that this file has the correct format
  • Check the size of the drive. If it’s larger than some arbitrary (configurable) value, give a warning and prompt the user to confirm before deploying
  • If the boot_out.txt file isn’t in the correct format, abort
  • If the boot_out.txt file isn’t found, give an error and a link to the MicroPython and/or CircuitPython Web sites

This seems like a relatively safe strategy in terms of preventing accidentally wiping out drive contents. I’m also going to see about extending Adafruit_Board_Toolkit to be able to do some more sophisticated board detection/enumeration, and then we could add an additional check for the target path being the correct drive type or something later. But I think this approach gives that margin of safety without compromising usability.

If this works for you, I’ll work on it and submit a PR when I’m done. In the meantime, can you assign this issue to be? For some reason, GitHub isn’t letting me do that myself.

@mraleson
Copy link
Owner

Works for me, thanks for taking this on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants