🔧 Reversible file alpha sorting (MarlinFirmware#26130)

Co-authored-by: Scott Lahteine <[email protected]>

Marlin/Configuration_adv.h

@@ -1746,9 +1746,10 @@
 
   // SD Card Sorting options
   #if ENABLED(SDCARD_SORT_ALPHA)
+    #define SDSORT_REVERSE     false  // Default to sorting file names in reverse order.
     #define SDSORT_LIMIT       40     // Maximum number of sorted items (10-256). Costs 27 bytes each.
-    #define FOLDER_SORTING     -1     // -1=above  0=none  1=below
-    #define SDSORT_GCODE       false  // Allow turning sorting on/off with LCD and M34 G-code.
+    #define SDSORT_FOLDERS     -1     // -1=above  0=none  1=below
+    #define SDSORT_GCODE       false  // Enable G-code M34 to set sorting behaviors: M34 S<-1|0|1> F<-1|0|1>
     #define SDSORT_USES_RAM    false  // Pre-allocate a static array for faster pre-sorting.
     #define SDSORT_USES_STACK  false  // Prefer the stack for pre-sorting to give back some SRAM. (Negated by next 2 options.)
     #define SDSORT_CACHE_NAMES false  // Keep sorted items in RAM longer for speedy performance. Most expensive option.

Marlin/src/gcode/sd/M34.cpp

@@ -29,9 +29,19 @@
 
 /**
  * M34: Set SD Card Sorting Options
+ *
+ * S   - Default sorting (i.e., SDSORT_REVERSE)
+ * S-1 - Reverse alpha sorting
+ * S0  - FID Order (not always newest)
+ * S1  - Forward alpha sorting
+ * S2  - Alias for S-1 [deprecated]
+ *
+ * F-1 - Folders above files
+ * F0  - Sort According to 'S'
+ * F1  - Folders after files
  */
 void GcodeSuite::M34() {
-  if (parser.seen('S')) card.setSortOn(parser.value_bool());
+  if (parser.seen('S')) card.setSortOn(SortFlag(parser.ushortval('S', TERN(SDSORT_REVERSE, AS_REV, AS_FWD))));
   if (parser.seenval('F')) {
     const int v = parser.value_long();
     card.setSortFolders(v < 0 ? -1 : v > 0 ? 1 : 0);

Marlin/src/inc/Changes.h

@@ -661,6 +661,8 @@
   #error "Z4_USE_ENDSTOP is obsolete. Instead set Z4_STOP_PIN directly. (e.g., 'Z4_USE_ENDSTOP _ZMAX_' becomes 'Z4_STOP_PIN Z_MAX_PIN')"
 #elif defined(INTEGRATED_BABYSTEPPING)
   #error "INTEGRATED_BABYSTEPPING is no longer needed and should be removed."
+#elif defined(FOLDER_SORTING)
+  #error "FOLDER_SORTING is now SDSORT_FOLDERS."
 #endif
 
 // L64xx stepper drivers have been removed

Marlin/src/inc/Conditionals_post.h

@@ -3337,8 +3337,8 @@
     #define SDSORT_CACHE_NAMES true
     #define SDSORT_CACHE_LPC1768_WARNING 1
   #endif
-  #ifndef FOLDER_SORTING
-    #define FOLDER_SORTING     -1
+  #ifndef SDSORT_FOLDERS
+    #define SDSORT_FOLDERS     -1
   #endif
   #ifndef SDSORT_GCODE
     #define SDSORT_GCODE       false

Marlin/src/lcd/e3v2/proui/dwin.cpp

@@ -850,7 +850,7 @@ bool DWIN_lcd_sd_status = false;
 #if ENABLED(MEDIASORT_MENU_ITEM)
   void setMediaSort() {
     toggleCheckboxLine(hmiData.mediaSort);
-    card.setSortOn(hmiData.mediaSort);
+    card.setSortOn(hmiData.mediaSort ? TERN(SDSORT_REVERSE, AS_REV, AS_FWD) : AS_OFF);
   }
 #endif
 
@@ -1754,7 +1754,7 @@ void dwinSetDataDefaults() {
   #endif
   #if ENABLED(MEDIASORT_MENU_ITEM)
     hmiData.mediaSort = true;
-    card.setSortOn(true);
+    card.setSortOn(TERN(SDSORT_REVERSE, AS_REV, AS_FWD));
   #endif
   hmiData.mediaAutoMount = ENABLED(HAS_SD_EXTENDER);
   #if ALL(INDIVIDUAL_AXIS_HOMING_SUBMENU, MESH_BED_LEVELING)

Marlin/src/sd/SdBaseFile.cpp

@@ -1422,11 +1422,13 @@ int16_t SdBaseFile::read(void * const buf, uint16_t nbyte) {
  *
  * \param[out] dir The dir_t struct that will receive the data.
  *
- * \return For success readDir() returns the number of bytes read.
- * A value of zero will be returned if end of file is reached.
- * If an error occurs, readDir() returns -1.  Possible errors include
- * readDir() called before a directory has been opened, this is not
- * a directory file or an I/O error occurred.
+ * \return For success return a non-zero value (number of bytes read).
+ *         A value of zero will be returned if end of dir is reached.
+ *         If an error occurs, readDir() returns -1. Possible errors:
+ *           - readDir() called on unopened dir
+ *           - not a directory file
+ *           - bad dir entry
+ *           - I/O error
  */
 int8_t SdBaseFile::readDir(dir_t * const dir, char * const longFilename) {
   int16_t n;
@@ -1488,7 +1490,7 @@ int8_t SdBaseFile::readDir(dir_t * const dir, char * const longFilename) {
                   longFilename[idx] = utf16_ch & 0xFF;
                   longFilename[idx + 1] = (utf16_ch >> 8) & 0xFF;
                 #else
-                  // Replace all multibyte characters to '_'
+                  // Replace multibyte character with '_'
                   longFilename[n + i] = (utf16_ch > 0xFF) ? '_' : (utf16_ch & 0xFF);
                 #endif
               }

Marlin/src/sd/cardreader.cpp

@@ -91,8 +91,8 @@ int16_t CardReader::nrItems = -1;
 
   int16_t CardReader::sort_count;
   #if ENABLED(SDSORT_GCODE)
-    bool CardReader::sort_alpha;
-    int CardReader::sort_folders;
+    SortFlag CardReader::sort_alpha;
+    int8_t CardReader::sort_folders;
     //bool CardReader::sort_reverse;
   #endif
 
@@ -160,8 +160,8 @@ CardReader::CardReader() {
   #if ENABLED(SDCARD_SORT_ALPHA)
     sort_count = 0;
     #if ENABLED(SDSORT_GCODE)
-      sort_alpha = true;
-      sort_folders = FOLDER_SORTING;
+      sort_alpha = TERN(SDSORT_REVERSE, AS_REV, AS_FWD);
+      sort_folders = SDSORT_FOLDERS;
       //sort_reverse = false;
     #endif
   #endif
@@ -993,7 +993,7 @@ void CardReader::selectFileByIndex(const int16_t nr) {
     if (nr < sort_count) {
       strcpy(filename, sortshort[nr]);
       strcpy(longFilename, sortnames[nr]);
-      flag.filenameIsDir = IS_DIR(nr);
+      TERN_(HAS_FOLDER_SORTING, flag.filenameIsDir = IS_DIR(nr));
       setBinFlag(strcmp_P(strrchr(filename, '.'), PSTR(".BIN")) == 0);
       return;
     }
@@ -1011,7 +1011,7 @@ void CardReader::selectFileByName(const char * const match) {
       if (strcasecmp(match, sortshort[nr]) == 0) {
         strcpy(filename, sortshort[nr]);
         strcpy(longFilename, sortnames[nr]);
-        flag.filenameIsDir = IS_DIR(nr);
+        TERN_(HAS_FOLDER_SORTING, flag.filenameIsDir = IS_DIR(nr));
         setBinFlag(strcmp_P(strrchr(filename, '.'), PSTR(".BIN")) == 0);
         return;
       }
@@ -1163,7 +1163,7 @@ void CardReader::cdroot() {
    * Get the name of a file in the working directory by sort-index
    */
   void CardReader::selectFileByIndexSorted(const int16_t nr) {
-    selectFileByIndex(TERN1(SDSORT_GCODE, sort_alpha) && (nr < sort_count) ? sort_order[nr] : nr);
+    selectFileByIndex(SortFlag(TERN1(SDSORT_GCODE, sort_alpha != AS_OFF)) && (nr < sort_count) ? sort_order[nr] : nr);
   }
 
   #if ENABLED(SDSORT_USES_RAM)
@@ -1210,7 +1210,7 @@ void CardReader::cdroot() {
     int16_t fileCnt = get_num_items();
 
     // Sorting may be turned off
-    if (TERN0(SDSORT_GCODE, !sort_alpha)) return;
+    if (TERN0(SDSORT_GCODE, sort_alpha == AS_OFF)) return;
 
     // If there are files, sort up to the limit
     if (fileCnt > 0) {
@@ -1239,9 +1239,9 @@ void CardReader::cdroot() {
         // Folder sorting needs 1 bit per entry for flags.
         #if HAS_FOLDER_SORTING
           #if ENABLED(SDSORT_DYNAMIC_RAM)
-            isDir = new uint8_t[(fileCnt + 7) >> 3];
+            isDir = new uint8_t[(fileCnt + 7) >> 3];  // Allocate space with 'new'
           #elif ENABLED(SDSORT_USES_STACK)
-            uint8_t isDir[(fileCnt + 7) >> 3];
+            uint8_t isDir[(fileCnt + 7) >> 3];        // Use stack in this scope
           #endif
         #endif
 
@@ -1291,18 +1291,18 @@ void CardReader::cdroot() {
             const int16_t o2 = sort_order[j + 1];
 
             // Compare names from the array or just the two buffered names
-            #if ENABLED(SDSORT_USES_RAM)
-              #define _SORT_CMP_NODIR() (strcasecmp(sortnames[o1], sortnames[o2]) > 0)
-            #else
-              #define _SORT_CMP_NODIR() (strcasecmp(name1, name2) > 0)
-            #endif
+            auto _sort_cmp_file = [](char * const n1, char * const n2) -> bool {
+              const bool sort = strcasecmp(n1, n2) > 0;
+              return (TERN(SDSORT_GCODE, card.sort_alpha == AS_REV, ENABLED(SDSORT_REVERSE))) ? !sort : sort;
+            };
+            #define _SORT_CMP_FILE() _sort_cmp_file(TERN(SDSORT_USES_RAM, sortnames[o1], name1), TERN(SDSORT_USES_RAM, sortnames[o2], name2))
 
             #if HAS_FOLDER_SORTING
               #if ENABLED(SDSORT_USES_RAM)
                 // Folder sorting needs an index and bit to test for folder-ness.
-                #define _SORT_CMP_DIR(fs) (IS_DIR(o1) == IS_DIR(o2) ? _SORT_CMP_NODIR() : IS_DIR(fs > 0 ? o1 : o2))
+                #define _SORT_CMP_DIR(fs) (IS_DIR(o1) == IS_DIR(o2) ? _SORT_CMP_FILE() : IS_DIR(fs > 0 ? o1 : o2))
               #else
-                #define _SORT_CMP_DIR(fs) ((dir1 == flag.filenameIsDir) ? _SORT_CMP_NODIR() : (fs > 0 ? dir1 : !dir1))
+                #define _SORT_CMP_DIR(fs) ((dir1 == flag.filenameIsDir) ? _SORT_CMP_FILE() : (fs > 0 ? dir1 : !dir1))
               #endif
             #endif
 
@@ -1318,12 +1318,12 @@ void CardReader::cdroot() {
             if (
               #if HAS_FOLDER_SORTING
                 #if ENABLED(SDSORT_GCODE)
-                  sort_folders ? _SORT_CMP_DIR(sort_folders) : _SORT_CMP_NODIR()
+                  sort_folders ? _SORT_CMP_DIR(sort_folders) : _SORT_CMP_FILE()
                 #else
-                  _SORT_CMP_DIR(FOLDER_SORTING)
+                  _SORT_CMP_DIR(SDSORT_FOLDERS)
                 #endif
               #else
-                _SORT_CMP_NODIR()
+                _SORT_CMP_FILE()
               #endif
             ) {
               // Reorder the index, indicate that sorting happened
@@ -1357,12 +1357,14 @@ void CardReader::cdroot() {
           #if ENABLED(SDSORT_DYNAMIC_RAM)
             sortnames = new char*[1];
             sortshort = new char*[1];
-            isDir = new uint8_t[1];
           #endif
           selectFileByIndex(0);
           SET_SORTNAME(0);
           SET_SORTSHORT(0);
-          isDir[0] = flag.filenameIsDir;
+          #if ALL(HAS_FOLDER_SORTING, SDSORT_DYNAMIC_RAM)
+            isDir = new uint8_t[1];
+            isDir[0] = flag.filenameIsDir;
+          #endif
         #endif
       }
 

Marlin/src/sd/cardreader.h

@@ -31,7 +31,10 @@ extern const char M23_STR[], M24_STR[];
   #if ENABLED(SDSORT_DYNAMIC_RAM)
     #define SD_RESORT 1
   #endif
-  #if FOLDER_SORTING || ENABLED(SDSORT_GCODE)
+  #ifndef SDSORT_FOLDERS
+    #define SDSORT_FOLDERS 0
+  #endif
+  #if SDSORT_FOLDERS || ENABLED(SDSORT_GCODE)
     #define HAS_FOLDER_SORTING 1
   #endif
 #endif
@@ -84,6 +87,7 @@ typedef struct {
 } card_flags_t;
 
 enum ListingFlags : uint8_t { LS_LONG_FILENAME, LS_ONLY_BIN, LS_TIMESTAMP };
+enum SortFlag : int8_t { AS_REV = -1, AS_OFF, AS_FWD, AS_ALSO_REV };
 
 #if ENABLED(AUTO_REPORT_SD_STATUS)
   #include "../libs/autoreport.h"
@@ -199,8 +203,8 @@ class CardReader {
     static void presort();
     static void selectFileByIndexSorted(const int16_t nr);
     #if ENABLED(SDSORT_GCODE)
-      FORCE_INLINE static void setSortOn(bool b)        { sort_alpha   = b; presort(); }
-      FORCE_INLINE static void setSortFolders(int i)    { sort_folders = i; presort(); }
+      FORCE_INLINE static void setSortOn(const SortFlag f) { sort_alpha = (f == AS_ALSO_REV) ? AS_REV : f; presort(); }
+      FORCE_INLINE static void setSortFolders(const int8_t i) { sort_folders = i; presort(); }
       //FORCE_INLINE static void setSortReverse(bool b) { sort_reverse = b; }
     #endif
   #else
@@ -272,12 +276,12 @@ class CardReader {
   #if ENABLED(SDCARD_SORT_ALPHA)
     static int16_t sort_count;    // Count of sorted items in the current directory
     #if ENABLED(SDSORT_GCODE)
-      static bool sort_alpha;     // Flag to enable / disable the feature
-      static int sort_folders;    // Folder sorting before/none/after
+      static SortFlag sort_alpha; // Sorting: REV, OFF, FWD
+      static int8_t sort_folders; // Folder sorting before/none/after
       //static bool sort_reverse; // Flag to enable / disable reverse sorting
     #endif
 
-    // By default the sort index is static
+    // By default the sort index is statically allocated
     #if ENABLED(SDSORT_DYNAMIC_RAM)
       static uint8_t *sort_order;
     #else

buildroot/tests/STM32F103VE_longer_maple

@@ -8,16 +8,16 @@ set -e
 
 use_example_configs Alfawise/U20
 opt_enable BAUD_RATE_GCODE
-exec_test $1 $2 "maple CLASSIC_UI U20 config" "$3"
+exec_test $1 $2 "Maple - Alfawise U20 - CLASSIC_UI" "$3"
 
 use_example_configs Alfawise/U20
 opt_enable BAUD_RATE_GCODE TFT_COLOR_UI
 opt_disable TFT_CLASSIC_UI CUSTOM_STATUS_SCREEN_IMAGE
-exec_test $1 $2 "maple COLOR_UI U20 config" "$3"
+exec_test $1 $2 "Maple - Alfawise U20 - COLOR_UI" "$3"
 
 use_example_configs Alfawise/U20-bltouch
 opt_enable BAUD_RATE_GCODE
-exec_test $1 $2 "maple BLTouch U20 config"
+exec_test $1 $2 "Maple - Alfawise U20 - BLTouch"
 
 # cleanup
 restore_configs