Buggy SD file management via OctoPrint

mujido commented 5 years ago

Remote file management on the SD card is quite broken. Filenames get out of sync with their respective files. For instance if I delete a gcode file and then add a new one, the on-printer menu will instead show a filename I deleted but it points to the new gcode if you actually try to print it from the printer menu. It's not just a single occurrence either; I removed multiple files and adding multiple files back on will resurrect the old names one-by-one but each file will point to one of my new files. Doing a reset or power restart does not fix the issue.

This does not happen if I manage the SD card directly on my PC. Deleting files and adding new files results in the same files appearing on the printer menu when reinserting the SD card.

mujido commented 5 years ago

Not sure if you guys have made any progress on this. I'm considering fixing it myself. I'm pretty sure the problem is that deleting files doesn't cleanup the long filename directory entries.

arekm commented 2 years ago

I can confirm this bug. Testing under current MK3 branch.

Reproduce steps:

fresh sd card, formatted under linux (mkfs.vfat /dev/sda1 in my case) or windows. Actually doesn't have to be fresh but...
copy long filename to it under linux/windows
put sd card into printer

Send: M20 L
Recv: Begin file list
Recv: PTFE_4~1.GCO 284654 "ptfe_4mm_cutting_guide_0.35mm_PLA__14m.gcode"
Recv: End file list
Recv: ok

delete file (click trash in octoprint for example)

Send: M30 /ptfe_4~1.gco
Recv: File deleted:ptfe_4~1.gco
Recv: ok
Send: M20 L T
Recv: Begin file list
Recv: End file list

upload file via serial (upload to sd in octoprint)

Send: M28 /autole~1.gco
Recv: echo:Now fresh file: /autole~1.gco
Recv: Writing to file: /ptfe_4mm_cutting_guide_0.35mm_PLA__14m.gcode
Changing monitoring state from "Operational" to "Sending file to SD"
Recv: File selected
Recv: LCD status changed
Recv: ok
Send: N1 G21*27
[...]
Send: N8 M29*16
Recv: Done saving file.

see above that it was saved under old long filename. Also confirm with M20 L T:

Send: M20 L T
Recv: Begin file list
Recv: AUTOLE~1.GCO 78 0x28210800 "ptfe_4mm_cutting_guide_0.35mm_PLA__14m.gcode"
Recv: End file list

LCD shows wrong long filename, too.

Another testing, auto.gco uploaded.

Mounted under linux:

# ls -l /media/
total 4
-rwxr-xr-x 1 root root 78 Jan  1  2000 AUTO.GCO

but

# fsck -vvvv /dev/sda1
fsck from util-linux 2.33.1
fsck.fat 4.1 (2017-01-24)
Checking we can access the last sector of the filesystem
Boot sector contents:
System ID "mkfs.fat"
Media byte 0xf8 (hard disk)
       512 bytes per logical sector
      4096 bytes per cluster
        32 reserved sectors
First FAT starts at byte 16384 (sector 32)
         2 FATs, 32 bit entries
   7745536 bytes per FAT (= 15128 sectors)
Root directory start at cluster 2 (arbitrary size)
Data area starts at byte 15507456 (sector 30288)
   1935670 data clusters (7928504320 bytes)
62 sectors/track, 245 heads
      8192 hidden sectors
  15515648 sectors total
Wrong checksum for long file name "ptfe_4mm_cutting_guide_0.35mm_PLA__14m.gcode".
  (Short name AUTO.GCO may have changed without updating the long name)
1: Delete LFN
2: Leave it as it is.
3: Fix checksum (attaches to short name AUTO.GCO)
? 1
Checking for unused clusters.
Checking free cluster summary.
Perform changes ? (y/n) y
/dev/sda1: 1 files, 2/1935670 clusters

so it was incorrectly stored in fat filesystem, not just in memory.

arekm commented 2 years ago

There is one fix in merlin repo that's not in prusa firmware but it seems to yet be another issue (doesn't fix problem from this issue for me): https://github.com/MarlinFirmware/Marlin/pull/9343

ellensp commented 2 years ago

"Send: M28 /autole~1.gco"

this test seems wrong.. should never directly use ~1 [2...etc] in a 8.3 name.

Your uploading a file name that tells the os there is a longer file name, but you never tell it the longer file name...

arekm commented 2 years ago

Hm, I see no such restriction. https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/18e63b13-ba43-4f5f-a5b7-11e871b71f14 and ~ is allowed according to https://en.wikipedia.org/wiki/8.3_filename

Anyway

Send: M28 /auto.gco
Recv: echo:Now fresh file: /auto.gco
Recv: Writing to file: /ptfe_4mm_cutting_guide_0.35mm_PLA__14m.gcode
Changing monitoring state from "Operational" to "Sending file to SD"
Recv: File selected
Recv: LCD status changed
Recv: ok
Send: N1 G21*27
[...]
Send: N8 M29*16
Recv: Done saving file.
Changing monitoring state from "Printing" to "Operational"
Send: M20 L T
Recv: Begin file list
Recv: AUTO.GCO 78 0x28210800 "ptfe_4mm_cutting_guide_0.35mm_PLA__14m.gcode"
Recv: End file list

ellensp commented 2 years ago

and if you upload a non 8.3 compliment file name, does that work as expected?

arekm commented 2 years ago

Firmware doesn't do any validation on name and long filename is still wrong even with non valid 8.3 name:

Send: M20 L T
Recv: Begin file list
Recv: 123:45.GCO 78 0x28210800 "ptfe_4mm_cutting_guide_0.35mm_PLA__14m.gcode"
Recv: End file list

":" is not allowed character but for example Linux handles it fine:

(mounted sd card here)
# ls /media/
123:45.GCO

Let see what fsck says about that:

# fsck -vvv /dev/sda1
fsck from util-linux 2.33.1
fsck.fat 4.1 (2017-01-24)
Checking we can access the last sector of the filesystem
Boot sector contents:
System ID "mkfs.fat"
Media byte 0xf8 (hard disk)
       512 bytes per logical sector
      4096 bytes per cluster
        32 reserved sectors
First FAT starts at byte 16384 (sector 32)
         2 FATs, 32 bit entries
   7745536 bytes per FAT (= 15128 sectors)
Root directory start at cluster 2 (arbitrary size)
Data area starts at byte 15507456 (sector 30288)
   1935670 data clusters (7928504320 bytes)
62 sectors/track, 245 heads
      8192 hidden sectors
  15515648 sectors total
Wrong checksum for long file name "ptfe_4mm_cutting_guide_0.35mm_PLA__14m.gcode".
  (Short name 123:45.GCO may have changed without updating the long name)
1: Delete LFN
2: Leave it as it is.
3: Fix checksum (attaches to short name 123:45.GCO)
? 1
/123:45.GCO
  Bad short file name (123:45.GCO).
1) Drop file
2) Rename file
3) Auto-rename
4) Keep it
? 3
  Renamed to FSCK0000.000
Checking for unused clusters.
Checking free cluster summary.
Perform changes ? (y/n) y
/dev/sda1: 1 files, 2/1935670 clusters

arekm commented 2 years ago

Marlin 2 got this covered recently (Jan 2022) in remove():

https://github.com/MarlinFirmware/Marlin/blob/bugfix-2.1.x/Marlin/src/sd/SdBaseFile.cpp#L1607-L1644

github-actions[bot] commented 1 year ago

This issue has been flagged as stale because it has been open for 60 days with no activity. The issue will be closed in 7 days unless someone removes the "stale" label or adds a comment.

github-actions[bot] commented 1 year ago

This issue has been closed due to lack of recent activity.

prusa3d / Prusa-Firmware

Buggy SD file management via OctoPrint #1495