Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Enabling LONG_FILENAME_WRITE_SUPPORT fails the compilation #23748

Closed
stro opened this issue Feb 16, 2022 · 6 comments
Closed

[BUG] Enabling LONG_FILENAME_WRITE_SUPPORT fails the compilation #23748

stro opened this issue Feb 16, 2022 · 6 comments

Comments

@stro
Copy link

stro commented Feb 16, 2022

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

Yes, and the problem still exists.

Bug Description

Enabling LONG_FILENAME_WRITE_SUPPORT in addition to LONG_FILENAME_HOST_SUPPORT fails with the following message:

Compiling .pio\build\STM32F103RE_creality\src\src\sd\cardreader.cpp.o
Marlin\src\sd\SdBaseFile.cpp: In static member function 'static void SdBaseFile::getLFNName(vfat_t*, char*, uint8_t)':
Marlin\src\sd\SdBaseFile.cpp:1143:9: error: 'longFilename' was not declared in this scope
1143 | longFilename[idx] = utf16_ch & 0xFF;
| ^~~~~~~~~~~~
*** [.pio\build\STM32F103RE_creality\src\src\sd\SdBaseFile.cpp.o] Error 1

That's because instead of "char *longFilename", the parameter is "char *lname"

Bug Timeline

e246d65ad1 Marlin/src/sd/SdBaseFile.cpp (GHGiampy 2022-01-18 07:56:11 +0100

Expected behavior

Successul compilation

Actual behavior

Compilation failure

Steps to Reproduce

No response

Version of Marlin Firmware

bugfix-2.0.x, 2022-02-14

Printer model

No response

Electronics

No response

Add-ons

No response

Bed Leveling

No response

Your Slicer

No response

Host Software

No response

Additional information & file uploads

No response

@ellensp
Copy link
Contributor

ellensp commented Feb 16, 2022

UTF_FILENAME_SUPPORT is also required to trigger this bug

@ellensp
Copy link
Contributor

ellensp commented Feb 16, 2022

Your fix is incorrect

code in question

  void SdBaseFile::getLFNName(vfat_t *pFatDir, char *lname, uint8_t sequenceNumber) {
    uint8_t startOffset = (sequenceNumber - 1) * FILENAME_LENGTH;
    LOOP_L_N(i, FILENAME_LENGTH) {
      const uint16_t utf16_ch = (i >= 11) ? pFatDir->name3[i - 11] : (i >= 5) ? pFatDir->name2[i - 5] : pFatDir->name1[i];
      #if ENABLED(UTF_FILENAME_SUPPORT)
        // We can't reconvert to UTF-8 here as UTF-8 is variable-size encoding, but joining LFN blocks
        // needs static bytes addressing. So here just store full UTF-16LE words to re-convert later.
        uint16_t idx = (startOffset + i) * 2; // This is fixed as FAT LFN always contain UTF-16LE encoding
        longFilename[idx] = utf16_ch & 0xFF;
        longFilename[idx + 1] = (utf16_ch >> 8) & 0xFF;
      #else
        // Replace all multibyte characters to '_'
        lname[startOffset + i] = (utf16_ch > 0xFF) ? '_' : (utf16_ch & 0xFF);
      #endif
    }
  }

Your fix will break the code when UTF_FILENAME_SUPPORT is disabled

a better fix is to change
longFilename[idx] = utf16_ch & 0xFF;
longFilename[idx + 1] = (utf16_ch >> 8) & 0xFF;
to
lname[idx] = utf16_ch & 0xFF;
lname[idx + 1] = (utf16_ch >> 8) & 0xFF;

@GHGiampy
Copy link
Contributor

Even better

lname[startOffset] = utf16_ch & 0xFF;
lname[startOffset + 1] = (utf16_ch >> 8) & 0xFF;

@ellensp
Copy link
Contributor

ellensp commented Feb 16, 2022

@GHGiampy really, when uint16_t idx = (startOffset + i) * 2; ?

I don't know if that line is correct, but if you just doing a simple substitution

lname[ (startOffset + i) * 2] = utf16_ch & 0xFF;
lname[((startOffset + i) * 2 )+ 1] = (utf16_ch >> 8) & 0xFF;

@GHGiampy
Copy link
Contributor

Sorry @ellensp, you are right, I missed idx declaration inside and think that was an copy/paste of an old block of code

@github-actions
Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants