add storage.erase_filesystem() to erase and reformat CIRCUITPY #747

dhalbert · 2018-04-09T17:00:23Z

Fixes #577.

Note that storage.erase_filesystem() does a microcontroller.reset() after reformatting. This is necessary to get the host computer synced up with the filesystem, including both contents and mountpoint name.

The main use of this is to be called from the REPL, since if the filesystem is damaged, it's not likely one can write a file to call this anyway.

I purposely chose a long name to make it less likely to be used casually.

#577 discusses adding storage.format(...), which could also be used on the SD card. That's a good idea, but a simple reformat of a filesystem visible to the host without syncing up leaves things in a bad state.

ladyada · 2018-04-09T17:58:08Z

this is rad!

jerryneedell · 2018-04-09T18:36:10Z

cool! worked great on a Trinket_m0!

jerryneedell · 2018-04-09T18:47:47Z

On itsybitsy_mo_express, it works, BUT - I have to press the reset button after erasing the FS to get it to boot. After erasing the FS it tries to reboot, but hangs with a blue dotstar.

jerryneedell · 2018-04-09T18:52:11Z

hmm - tried itsybitsy again it it worked properly (auto rebooted).

jerryneedell · 2018-04-09T19:14:11Z

Now it is back to failing regularly on my itsybitsy m0 espress.

jerryneedell · 2018-04-09T19:22:12Z

nevermind - false alarm on the itsybitys - it is working . I got my branches confused...

jerryneedell · 2018-04-09T19:43:33Z

I am getting to following warning when compiling ```
jerryneedell@Ubuntu-Macmini:~/projects/adafruit_github/circuitpython_master/ports/atmel-samd$ make BOARD=itsybitsy_m0_express
Use make V=1, make V=2 or set BUILD_VERBOSE similarly in your environment to increase build verbosity.
install -d build-itsybitsy_m0_express/genhdr
python3 tools/gen_usb_descriptor.py
--manufacturer "Adafruit Industries LLC"
--product "Itsy Bitsy M0 Express"
--vid 0x239A
--pid 0x8012
--output_c_file build-itsybitsy_m0_express/autogen_usb_descriptor.c
--output_h_file build-itsybitsy_m0_express/genhdr/autogen_usb_descriptor.h
Generating build-itsybitsy_m0_express/genhdr/mpversion.h
GEN build-itsybitsy_m0_express/genhdr/qstr.i.last
QSTR updated
GEN build-itsybitsy_m0_express/genhdr/qstrdefs.generated.h
../../shared-bindings/usb_hid/init.h:33:23: warning: size of 'common_hal_usb_hid_devices' differ from the size of original declaration [-Wlto-type-mismatch]
extern mp_obj_tuple_t common_hal_usb_hid_devices;
^
common-hal/usb_hid/init.c:83:16: note: 'common_hal_usb_hid_devices' was previously declared here
mp_obj_tuple_t common_hal_usb_hid_devices = {
^

51780 bytes free in flash out of 253440 bytes ( 247.5 kb ).
4152 bytes free in ram for stack out of 32768 bytes ( 32.0 kb ).

Create build-itsybitsy_m0_express/firmware.bin
Create build-itsybitsy_m0_express/firmware.uf2
python2 ../../tools/uf2/utils/uf2conv.py -b 0x2000 -c -o build-itsybitsy_m0_express/firmware.uf2 build-itsybitsy_m0_express/firmware.bin
Converting to uf2, output size: 403456, start address: 0x2000
Wrote 403456 bytes to build-itsybitsy_m0_express/firmware.uf2.

dhalbert · 2018-04-09T19:56:47Z

@Jerryn Don't worry about this warning. -flto has a spurious warning bug. It's fixed in the next version of gcc. I changed how the USB device structure is declared at compile time, and it brought this back.

jerryneedell · 2018-04-09T20:02:03Z

Ah - Thanks for the reminder - I had forgotten about this. I think I am going to get a lot of mileage out of this PR!

jerryneedell · 2018-04-09T20:22:06Z

on the metro_m4_express_revb - the FS gets erased, but my linux box gets upset when it trys to unmount the CIRCUITPY dirve

[  318.707071] usb 3-3.4: output irq status -32 received
[  318.718099] cdc_acm 3-3.4:1.1: urb 8 failed submission with -2
[  318.718125] xhci_hcd 0000:00:14.0: WARN Cannot submit Set TR Deq Ptr
[  318.718130] xhci_hcd 0000:00:14.0: A Set TR Deq Ptr command is pending.
[  319.462709] usb 3-3.4: reset full-speed USB device number 8 using xhci_hcd
[  319.584665] cdc_acm 3-3.4:1.0: ttyACM0: USB ACM device

Rebooting the linux box clears it up, but that is annoying ;-)

dhalbert · 2018-04-09T20:26:52Z

@Jerryn You mean the USB port becomes unusable? The reset done by erase_filesystem() is basically the same as pulling the plug or pressing the reset button. When I tried it on Ubuntu it just remounted CIRCUITPY, and I was able to get into the REPL again. Is this on Raspbian?

jerryneedell · 2018-04-09T20:30:14Z

This is on Ubuntu 16.04

jerryneedell · 2018-04-09T20:34:21Z

just tried again - it erases the FS but CIRCUITPY is not unmounted - it still shows the original FS - If I unmount I cannot remount it. reseting the M4 results in the errors above. Rebooting the Ubuntu machine brings it all back.

jerryneedell · 2018-04-09T20:45:06Z

Tried it on a metro_m0_express - worked fine.

jerryneedell · 2018-04-09T21:24:38Z

BTW - this PR breaks the esp8266

FREEZE modules
build/shared-module/storage/__init__.o:(.text.common_hal_storage_erase_filesystem+0x0): undefined reference to `filesystem_init'
build/shared-module/storage/__init__.o: In function `common_hal_storage_erase_filesystem':
__init__.c:(.text.common_hal_storage_erase_filesystem+0xd): undefined reference to `filesystem_init'
Makefile:249: recipe for target 'build/firmware.elf' failed
make: *** [build/firmware.elf] Error 1

It builds OK on the nrf52 (feather52) - I have not tried the erase_filesystem() yet on it.

tannewt

Overall looks good but please fix the esp8266 build. You'll likely need to update the SDK too. Instructions to do so are here: https://github.com/pfalcon/esp-open-sdk#pulling-updates

tannewt · 2018-04-09T22:53:11Z

ports/atmel-samd/supervisor/filesystem.c

@@ -50,7 +50,7 @@ static void make_empty_file(FATFS *fatfs, const char *path) {

 // we don't make this function static because it needs a lot of stack and we
 // want it to be executed without using stack within main() function
-void filesystem_init(bool create_allowed) {
+void filesystem_init(bool create_allowed, bool create_force) {


Nitpick but I think this will read better as force_create.

dhalbert · 2018-04-10T00:49:28Z

@jerryneedell I am also using Ubuntu 16.04 x64, and don't see this with the M4 on my machine (a vanilla Dell Optiplex, 2013 or so). So it may depend on the low-level USB drivers or might be a timing problem.

I don't necessarily think this will help, but could you try adding a 1-second delay before resetting, in ports/atmel_samd/shared-module/storage/__Init__.c ?

At the top:

#include "py/mphal.h"

at the end of the file, add a delay call:

void common_hal_storage_erase_filesystem(void) {
    filesystem_init(false, true);   // Force a re-format.
    mp_hal_delay_ms(1000);
    common_hal_mcu_reset();
    // We won't actually get here, since we're resetting.
}

If that doesn't help, could you try a USB3 vs USB2 port, and/or a USB hub?

How are you mounting CIRCUITPY when it reboots? I am just clicking CIRCUITPY in a Nautilus windows (or the equivalent); sometimes I don't even need to do that: it's already mounted. I don't use command line mount.

jerryneedell · 2018-04-10T01:38:29Z

adding the delay did not have any impact, but connecting the M4 to a USB port directly on the Host machine (no hub) does work.

also as suggested by Dan, just power cycling the USB Hub will also successfully remount CIRCUITPY

jerryneedell · 2018-04-10T02:08:59Z

Tried on M4 revb on MacOS -- remounts successfully - even on USB2 Hub
Tried on Raspian to Pi 3B+ - fails to unmount just as Ubuntu - power cycling hub remounts it successfully.

This caused a fatal compiler diagnostic after adafruit#750. This compiler flag is already specified in the atmel-samd builds, so it makes sense to do it here for the same reasons.

dhalbert · 2018-04-10T16:10:48Z

On ESP8266: throw NotImplementedError. It would be possible to erase the filesystem but then the needed boot.py would not be present. It's easier to just erase flash and re-upload CircuitPython. The NotImplementedError suggests this.

On nrf: placeholder NotImplementedError for now.

tannewt · 2018-04-10T17:26:02Z

Raising NotImplementedError makes sense to me because the ESP8266 is much less likely to have corruption since it doesn't do USB mass storage.

add storage.erase_filesystem() to erase and reformat CIRCUITPY

aa8c262

dhalbert requested a review from tannewt April 9, 2018 17:00

dhalbert closed this Apr 9, 2018

dhalbert reopened this Apr 9, 2018

tannewt requested changes Apr 9, 2018

View reviewed changes

jepler and others added 5 commits April 9, 2018 22:23

Create genhdr/ directory in time

eb7d0e3

Prevent a heap pointer from living on the stack longer than we need.

3b511e5

Fix rom qstr pool length.

fd70383

esp8266: Disable "strict aliasing" in compiler like in atmel-samd

1e9a271

This caused a fatal compiler diagnostic after adafruit#750. This compiler flag is already specified in the atmel-samd builds, so it makes sense to do it here for the same reasons.

esp8266 and nrf: raise NotImplementedError

5f98953

remove unnecessary includes in esp8266 common-hal storage code

04b2c8b

dhalbert added 3 commits April 10, 2018 12:16

remove more unneeded includes

80fb61e

Merge remote-tracking branch 'adafruit/master' into reformat

1b12c42

force_create rename got lost due to editing error

eaa9923

tannewt approved these changes Apr 10, 2018

View reviewed changes

tannewt merged commit f02cbea into adafruit:master Apr 10, 2018

dhalbert deleted the reformat branch May 4, 2018 20:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add storage.erase_filesystem() to erase and reformat CIRCUITPY #747

add storage.erase_filesystem() to erase and reformat CIRCUITPY #747

dhalbert commented Apr 9, 2018

ladyada commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

dhalbert commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

dhalbert commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018 •

edited

Loading

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018 •

edited

Loading

tannewt left a comment

tannewt Apr 9, 2018

dhalbert Apr 10, 2018

dhalbert commented Apr 10, 2018

jerryneedell commented Apr 10, 2018 •

edited

Loading

jerryneedell commented Apr 10, 2018

dhalbert commented Apr 10, 2018

tannewt commented Apr 10, 2018

add storage.erase_filesystem() to erase and reformat CIRCUITPY #747

add storage.erase_filesystem() to erase and reformat CIRCUITPY #747

Conversation

dhalbert commented Apr 9, 2018

ladyada commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

dhalbert commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

dhalbert commented Apr 9, 2018

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018 • edited Loading

jerryneedell commented Apr 9, 2018

jerryneedell commented Apr 9, 2018 • edited Loading

tannewt left a comment

Choose a reason for hiding this comment

tannewt Apr 9, 2018

Choose a reason for hiding this comment

dhalbert Apr 10, 2018

Choose a reason for hiding this comment

dhalbert commented Apr 10, 2018

jerryneedell commented Apr 10, 2018 • edited Loading

jerryneedell commented Apr 10, 2018

dhalbert commented Apr 10, 2018

tannewt commented Apr 10, 2018

jerryneedell commented Apr 9, 2018 •

edited

Loading

jerryneedell commented Apr 9, 2018 •

edited

Loading

jerryneedell commented Apr 10, 2018 •

edited

Loading