search

adafruit / Adafruit_CircuitPython_BitmapSaver

Save displayio bitmaps to BMP disk
MIT License
5 stars 9 forks source link

Update examples with umount #28

Closed DJDevon3 closed 1 year ago

DJDevon3 commented 1 year ago

Help avoid data corruption by unmounting SD card after use

tekktrik commented 1 year ago

This looks good to me, did you test these as well?

DJDevon3 commented 1 year ago

@tekktrik Only tested the TFT featherwing example and can confirm it works well. The other 2 I did just because I was in there. Should I not submit things I cannot explicitly test? It seemed like a shame to leave the other 2 examples alone without umount. I probably should have specified that in the original commit sorry.

tekktrik commented 1 year ago

No, totally fine to PR things you haven't tested! Just wanted to check, definitely always makes me more confident when it is tested is all :)

tekktrik commented 1 year ago

I'm out today but will look tomorrow!

DJDevon3 commented 1 year ago

Take screenshot is False by default. Set to true to take a screenshot.

A good real world example is at the very end of my feather weather project. That's my old "YOLO version" as Anecdata puts it where I race a 120 second timer to remove the SD card, upload it to the PC, and put it back. Still have to update my own scripts.

I included the if statement because I believe its in everyone's best interest to become accustomed to using it in that way, especially beginners.

Technically it doesn't need to be there... but then neither does umount. It adds safety measures so you don't

  1. take a screenshot when you don't mean to. Depending on screen size and MCU power this can take up to 5 minutes and you cannot back out once started. Conversely, depending on the speed of your MCU (esp32-s3 or iMX) and a tiny display you might get into a loop of never ending screenshot taking and by attempting to stop it mid-stream you risk data corruption.
  2. possibly corrupt data because you're unaware how important umount is.

Stopping an SD write mid-stream won't just corrupt the file you're trying to write, it will create gibberish files that cannot be deleted and/or corrupt your entire SD card.

Between the if statement and umount the risk of data corruption and infinite screenshot taking loop is virtually eliminated. For advanced users with never ending scripts at some point you'll likely want to remount but that is unnecessary in the simpletest examples. umount however is definitely necessary.

They're features I wish would have been included in the examples from the beginning. I've used this library more than anyone else I know. I've used it on the NRF52840, M0's, M4's, and ESP32-S2 & S3. Beginners in particular will run into data corruption issues if those 2 features are not included. I learned the hard way, these features are necessary.

DJDevon3 commented 1 year ago

@FoamyGuy that's a great point and points out a flaw in the way I've been using it. Will take another shot at improving it. Thank you for the excellent guidance.

DJDevon3 commented 1 year ago

Moved SD Card initialization into take screenshot if statement. Works well. Tested the bitmapsaver_screenshot_simpletest.py & bitmapsaver_screenshot_tft_featherwing.py. example successfully

screenshot

was unable to successfully test the bitmapsaver_simpletest.py example on my tft featherwing because sck in use. if a reviewer could test/confirm that works then push it. it just won't work by default on a 3.5" tft featherwing which is why we have a separate example for the tft featherwing. :) I'm not sure what hardware it's designed for so I don't want to mess with that one too much since I can't successfully test for it.