MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.17k stars 19.21k forks source link

[BUG] Selecting file for printng with `M23` by it's long filename does not work #25497

Closed eduard-sukharev closed 1 year ago

eduard-sukharev commented 1 year ago

Did you test the latest bugfix-2.1.x code?

Yes, and the problem still exists.

Bug Description

Even with LONG_FILENAME_HOST_SUPPORT and LONG_FILENAME_WRITE_SUPPORT defines, selecting a file for printing with M23 by the long file name does not work.

Bug Timeline

No response

Expected behavior

Selecting a file for printing with M23 works

Actual behavior

Selecting a file by it's long filename fails

Steps to Reproduce

  1. Insert SD card with a file that has long filename (non-DOS compatible), e.g. longfilename.gcode
  2. In Serial console issue M21 command
  3. In Serial console issue M20 L command. Verify that the file is there and reported with both DOS filename and Long filename (LONGFI~1.GCO and longfilename.gcode)
  4. In Serial console issue M23 longfilename.gcode to select the file

Version of Marlin Firmware

bugfix-2.1.x

Printer model

No response

Electronics

No response

Add-ons

No response

Bed Leveling

None

Your Slicer

None

Host Software

Pronterface

Don't forget to include

Additional information & file uploads

I have added a bunch of DEBUG_ECHO* calls that mostly duplicate code comments, into SdBaseFile.cpp, inside the method with following signature:

// open with filename in dname and long filename in dlname
bool SdBaseFile::open(SdBaseFile *dirFile, const uint8_t dname[11]
    OPTARG(LONG_FILENAME_WRITE_SUPPORT, const uint8_t dlname[LONG_FILENAME_LENGTH])
  , uint8_t oflag
)

and here's the sample serial console log:

ttyACM0 21°> M20 L
SENDING:M20 L
Begin file list
LONGF~1.GCO 758868 LONGF~1.GCO
LONGFI~1.GCO 902501 longfilename-1.gcode
LONGFI~2.GCO 879788 longfilename-2.gcode
SAMPLE~1.GCO 921715 sample_long_name.gcode
TEMP.GCO 444974 TEMP.GCO
End file list
ttyACM0 21°> M23 /longfilename-1.gcode
SENDING:M23 /LONGFILENAME-1.GCODE
Now fresh file: /LONGFILENAME-1.GCODE
>>> diveToFile  X0.00 Y208.00 Z0.00
 path = '/LONGFILENAME-1.GCODE'
 CWD to root: 0x200032D4
 startDirPtr = 0x200032D4
 final workDir = 0x200032D4
 returning string LONGFILENAME-1.GCODE
<<< diveToFile  X0.00 Y208.00 Z0.00
>>> SdBaseFile::open()  X0.00 Y208.00 Z0.00
 dname: LONGF~18GCO
 reqEntriesNum: 3 lfnNameLength: 20
 search for file
/  absolute index position: 0
  p->name: PRINT
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     useLFN
      !lfnFileFound
/  absolute index position: 1
  p->name: LONGF~1 GCO
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     useLFN
      !lfnFileFound
/  absolute index position: 2
  p->name: B1
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     useLFN
      !lfnFileFound
       isDirLFN(p)
       lfnSequenceNumber: 2
        Check checksum for all other entries with the starting checksum fetched before
         Set chunk of LFN from VFAT entry into lfnName
         LFN found?:          lfnFileFound: 0
/  absolute index position: 3
  p->name: l
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     useLFN
      !lfnFileFound
       isDirLFN(p)
       lfnSequenceNumber: 1
        Check checksum for all other entries with the starting checksum fetched before
         Set chunk of LFN from VFAT entry into lfnName
         LFN found?:          lfnFileFound: 0
/  absolute index position: 4
  p->name: LONGFI~1GCO
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     useLFN
      !lfnFileFound
/  absolute index position: 5
  p->name: B2
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     useLFN
      !lfnFileFound
       isDirLFN(p)
       lfnSequenceNumber: 2
        Check checksum for all other entries with the starting checksum fetched before
         Set chunk of LFN from VFAT entry into lfnName
         LFN found?:          lfnFileFound: 0
/  absolute index position: 6
  p->name: l
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     useLFN
      !lfnFileFound
       isDirLFN(p)
       lfnSequenceNumber: 1
        Check checksum for all other entries with the starting checksum fetched before
         Set chunk of LFN from VFAT entry into lfnName
         LFN found?:          lfnFileFound: 0
/  absolute index position: 7
  p->name: LONGFI~2GCO
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     useLFN
      !lfnFileFound
/  absolute index position: 8
  p->name: Ba
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     useLFN
      !lfnFileFound
       isDirLFN(p)
       lfnSequenceNumber: 2
        Check checksum for all other entries with the starting checksum fetched before
         Set chunk of LFN from VFAT entry into lfnName
         LFN found?:          lfnFileFound: 0
/  absolute index position: 9
  p->name: s
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     useLFN
      !lfnFileFound
       isDirLFN(p)
       lfnSequenceNumber: 1
        Check checksum for all other entries with the starting checksum fetched before
         Set chunk of LFN from VFAT entry into lfnName
         LFN found?:          lfnFileFound: 0
/  absolute index position: 10
  p->name: SAMPLE~1GCO
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     useLFN
      !lfnFileFound
/  absolute index position: 11
  p->name: TEMP    GCO
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     useLFN
      !lfnFileFound
/  absolute index position: 12
  p->name:
  Check empty status: Entry empty
<<< SdBaseFile::open()  X0.00 Y208.00 Z0.00
open failed, File: LONGFILENAME-1.GCODE.
ttyACM0 21°> M23 /LONGFI~1.GCO
SENDING:M23 /LONGFI~1.GCO
Now fresh file: /LONGFI~1.GCO
>>> diveToFile  X0.00 Y208.00 Z0.00
 path = '/LONGFI~1.GCO'
 CWD to root: 0x200032D4
 startDirPtr = 0x200032D4
 final workDir = 0x200032D4
 returning string LONGFI~1.GCO
<<< diveToFile  X0.00 Y208.00 Z0.00
>>> SdBaseFile::open()  X0.00 Y208.00 Z0.00
 dname: LONGFI~1GCO
 reqEntriesNum: 1 lfnNameLength: 0
 search for file
/  absolute index position: 0
  p->name: PRINT
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     !useLFN
/  absolute index position: 1
  p->name: LONGF~1 GCO
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     !useLFN
/  absolute index position: 2
  p->name: B1
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     !useLFN
/  absolute index position: 3
  p->name: l
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     !useLFN
/  absolute index position: 4
  p->name: LONGFI~1GCO
  Check empty status: Entry not empty
    Reset empty counter
    Search for SFN or LFN?
     !useLFN
      fileFound: 1
<<< SdBaseFile::open()  X0.00 Y208.00 Z0.00
File opened: LONGFI~1.GCO Size: 902501
File selected
ttyACM0 21°>

As can be clearly seen in

>>> SdBaseFile::open()  X0.00 Y208.00 Z0.00
 dname: LONGF~18GCO

the short filename composed to lookup the file by was wrong (and presumably pretty much random), which didn't match the actual short name of a file and thus selecting a file failed.

eduard-sukharev commented 1 year ago

Further debugging reveals following: in a file Marlin/src/sd/SdBaseFile.cpp a function with signature bool SdBaseFile::open(SdBaseFile *dirFile, const uint8_t dname[11] OPTARG(LONG_FILENAME_WRITE_SUPPORT, const uint8_t dlname[LONG_FILENAME_LENGTH]), uint8_t oflag) actually scans VFAT sequences to compose a complete LFN by matching LFN checksums, which means that it supposed to not care if the generated short dosname is random. The issue with mismatching filenames remains unresolved, though, as I still cannot understand why exactly that happens

eduard-sukharev commented 1 year ago

The issue seem to be that lfn comparison is done like this:

if (!strncasecmp((char*)dlname, (char*)lfnName, lfnNameLength)) lfnFileFound = true;

and while dlname and lfname are both uint8_t and have same length, dlname content is encoded as single-byte chars string (ASCII), while lfname contains two-byte chars encoding (UTF-16LE).

thisiskeithb commented 1 year ago

Closing since you've opened a PR.

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.