Add SD card validation step to client-side installer #363
Conversation
| let result = await printer.sshClient.execCommand("SDCARDDEV=$(df -Th /mnt/sdcard | awk 'NR==2 {print $1}') ; fsck.vfat -n $SDCARDDEV | grep -q FAT32"); | ||
| if (result.code != 0) { | ||
| console.log("SD card is NOT FAT32 formatted!"); | ||
| throw `SD card is NOT FAT32 formatted!`; |
There was a problem hiding this comment.
we should not just error, we should tell the user what to do. use the same language as installer/install.py
There was a problem hiding this comment.
Good catch, I've fixed this in a4a0db9
There was a problem hiding this comment.
that commit does not appear to be on the branch to pull?
There was a problem hiding this comment.
fsck.vfat is not really the way to do this (blkid probably is fine), but more to the point, this does not actually check the same thing as install.py, nor does it check the partition table; this check could pass and install.py could still reject the SD card.
relatedly, this seems like the sort of thing that we could detect before tripping on the installer? it seems in particular like the sort of thing that could error in connectSsh, before the user even gets as far as clicking install and then having to exit and restart the installer to recover...
There was a problem hiding this comment.
that commit does not appear to be on the branch to pull?
My bad, committed it to a different branch on accident. Good catch! Fixed with b01b9e4
There was a problem hiding this comment.
fsck.vfat is not really the way to do this (
blkidprobably is fine), but more to the point, this does not actually check the same thing asinstall.py, nor does it check the partition table; this check could pass andinstall.pycould still reject the SD card.relatedly, this seems like the sort of thing that we could detect before tripping on the installer? it seems in particular like the sort of thing that could error in
connectSsh, before the user even gets as far as clicking install and then having to exit and restart the installer to recover...
The check here is basically identical? validate_sd() checks if the filesystem is vfat. The command I wrote goes a step further and confirms that the fs is FAT32, as vfat applies to FAT32, FAT16, etc.
We could check earlier in the install process, but adding it here offered the cleanest drop-in of new code, without affecting anything else.
|
LGTM but one small bit of feedback |
|
@jwise ready to merge unless you want me to add formatting as a hidden advanced object while I'm working on this |
This PR add a new step to the client-side installer. The new step does the following:
This PR could be extended to implement #107, but if that is a no-go then that issue can be closed as rejected with the merge of this PR in its current state.
I have tested the new installer and confirmed that it's working.