Bug fixes (#790)

* Fix memory leak when answering webcommand (regression issue)

Fix wrong error code on  web command (regression issue)

* Fix sd code not disabled when SD Card feature is disabled

* Use proper error codes / messages

* Update Grbl.h

* Revert "Use proper error codes / messages"

This reverts commit ad49cf8cc1.

* Updated error code symbols and text

- To be generic file system error

Co-authored-by: Luc <luc.@tech-hunters.com>
Co-authored-by: bdring <barton.dring@gmail.com>

Grbl_Esp32/src/Error.cpp

@@ -61,16 +61,16 @@ std::map<Error, const char*> ErrorNames = {
     { Error::GcodeG43DynamicAxisError, "Gcode G43 dynamic axis error" },
     { Error::GcodeMaxValueExceeded, "Gcode max value exceeded" },
     { Error::PParamMaxExceeded, "P param max exceeded" },
-    { Error::SdFailedMount, "SD failed mount" },
+    { Error::FsFailedMount, "Failed to mount device" },
-    { Error::SdFailedRead, "SD failed read" },
+    { Error::FsFailedRead, "Failed to read" },
-    { Error::SdFailedOpenDir, "SD failed to open directory" },
+    { Error::FsFailedOpenDir, "Failed to open directory" },
-    { Error::SdDirNotFound, "SD directory not found" },
+    { Error::FsDirNotFound, "Directory not found" },
-    { Error::SdFileEmpty, "SD file empty" },
+    { Error::FsFileEmpty, "File empty" },
-    { Error::SdFileNotFound, "SD file not found" },
+    { Error::FsFileNotFound, "File not found" },
-    { Error::SdFailedOpenFile, "SD failed to open file" },
+    { Error::FsFailedOpenFile, "Failed to open file" },
-    { Error::SdFailedBusy, "SD is busy" },
+    { Error::FsFailedBusy, "Device is busy" },
-    { Error::SdFailedDelDir, "SD failed to delete directory" },
+    { Error::FsFailedDelDir, "Failed to delete directory" },
-    { Error::SdFailedDelFile, "SD failed to delete file" },
+    { Error::FsFailedDelFile, "Failed to delete file" },
     { Error::BtFailBegin, "Bluetooth failed to start" },
     { Error::WifiFailBegin, "WiFi failed to start" },
     { Error::NumberRange, "Number out of range for setting" },
@@ -79,5 +79,5 @@ std::map<Error, const char*> ErrorNames = {
     { Error::NvsSetFailed, "Failed to store setting" },
     { Error::NvsGetStatsFailed, "Failed to get setting status" },
     { Error::AuthenticationFailed, "Authentication failed!" },
-    { Error::AnotherInterfaceBusy, "Another interface is busy"},
+    { Error::AnotherInterfaceBusy, "Another interface is busy" },
 };

Grbl_Esp32/src/Error.h

@@ -64,16 +64,16 @@ enum class Error : uint8_t {
     GcodeG43DynamicAxisError    = 37,
     GcodeMaxValueExceeded       = 38,
     PParamMaxExceeded           = 39,
-    SdFailedMount               = 60,  // SD Failed to mount
+    FsFailedMount               = 60,  // SD Failed to mount
-    SdFailedRead                = 61,  // SD Failed to read file
+    FsFailedRead                = 61,  // SD Failed to read file
-    SdFailedOpenDir             = 62,  // SD card failed to open directory
+    FsFailedOpenDir             = 62,  // SD card failed to open directory
-    SdDirNotFound               = 63,  // SD Card directory not found
+    FsDirNotFound               = 63,  // SD Card directory not found
-    SdFileEmpty                 = 64,  // SD Card directory not found
+    FsFileEmpty                 = 64,  // SD Card directory not found
-    SdFileNotFound              = 65,  // SD Card file not found
+    FsFileNotFound              = 65,  // SD Card file not found
-    SdFailedOpenFile            = 66,  // SD card failed to open file
+    FsFailedOpenFile            = 66,  // SD card failed to open file
-    SdFailedBusy                = 67,  // SD card is busy
+    FsFailedBusy                = 67,  // SD card is busy
-    SdFailedDelDir              = 68,
+    FsFailedDelDir              = 68,
-    SdFailedDelFile             = 69,
+    FsFailedDelFile             = 69,
     BtFailBegin                 = 70,  // Bluetooth failed to start
     WifiFailBegin               = 71,  // WiFi failed to start
     NumberRange                 = 80,  // Setting number range problem

Grbl_Esp32/src/Grbl.h

@@ -22,7 +22,7 @@
 
 // Grbl versioning system
 const char* const GRBL_VERSION       = "1.3a";
-const char* const GRBL_VERSION_BUILD = "20210228";
+const char* const GRBL_VERSION_BUILD = "20210304";
 
 //#include <sdkconfig.h>
 #include <Arduino.h>

Grbl_Esp32/src/Report.cpp

@@ -291,10 +291,14 @@ std::map<Message, const char*> MessageText = {
 // NOTE: For interfaces, messages are always placed within brackets. And if silent mode
 // is installed, the message number codes are less than zero.
 void report_feedback_message(Message message) {  // ok to send to all clients
+#if defined (ENABLE_SD_CARD) 
     if (message == Message::SdFileQuit) {
         grbl_notifyf("SD print canceled", "Reset during SD file at line: %d", sd_get_current_line_number());
         grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Info, "Reset during SD file at line: %d", sd_get_current_line_number());
-    } else {
+
+    } else 
+#endif //ENABLE_SD_CARD  
+    {
         auto it = MessageText.find(message);
         if (it != MessageText.end()) {
             grbl_msg_sendf(CLIENT_SERIAL, MsgLevel::Info, it->second);

Grbl_Esp32/src/SDCard.cpp

@@ -18,7 +18,9 @@
   along with Grbl.  If not, see <http://www.gnu.org/licenses/>.
 */
 
-#include "SDCard.h"
+#include "Config.h"
+#ifdef ENABLE_SD_CARD
+#    include "SDCard.h"
 
 File                       myFile;
 bool                       SD_ready_next = false;  // Grbl has processed a line and is waiting for another
@@ -31,7 +33,7 @@ static char                comment[LINE_BUFFER_SIZE];  // Line to be executed. Z
 /*bool sd_mount()
 {
   if(!SD.begin()) {
-    report_status_message(Error::SdFailedMount, CLIENT_SERIAL);
+    report_status_message(Error::FsFailedMount, CLIENT_SERIAL);
     return false;
   }
   return true;
@@ -41,11 +43,11 @@ void listDir(fs::FS& fs, const char* dirname, uint8_t levels, uint8_t client) {
     //char temp_filename[128]; // to help filter by extension	TODO: 128 needs a definition based on something
     File root = fs.open(dirname);
     if (!root) {
-        report_status_message(Error::SdFailedOpenDir, client);
+        report_status_message(Error::FsFailedOpenDir, client);
         return;
     }
     if (!root.isDirectory()) {
-        report_status_message(Error::SdDirNotFound, client);
+        report_status_message(Error::FsDirNotFound, client);
         return;
     }
     File file = root.openNextFile();
@@ -64,7 +66,7 @@ void listDir(fs::FS& fs, const char* dirname, uint8_t levels, uint8_t client) {
 boolean openFile(fs::FS& fs, const char* path) {
     myFile = fs.open(path);
     if (!myFile) {
-        //report_status_message(Error::SdFailedRead, CLIENT_SERIAL);
+        //report_status_message(Error::FsFailedRead, CLIENT_SERIAL);
         return false;
     }
     set_sd_state(SDState::BusyPrinting);
@@ -94,7 +96,7 @@ boolean closeFile() {
 */
 boolean readFileLine(char* line, int maxlen) {
     if (!myFile) {
-        report_status_message(Error::SdFailedRead, SD_client);
+        report_status_message(Error::FsFailedRead, SD_client);
         return false;
     }
     sd_current_line_number += 1;
@@ -169,3 +171,4 @@ void sd_get_current_filename(char* name) {
         name[0] = 0;
     }
 }
+#endif  //ENABLE_SD_CARD

Grbl_Esp32/src/Serial.cpp

@@ -159,16 +159,20 @@ void serialCheckTask(void* pvParameters) {
             if (is_realtime_command(data)) {
                 execute_realtime_command(static_cast<Cmd>(data), client);
             } else {
+#if defined(ENABLE_SD_CARD)
                 if (get_sd_state(false) < SDState::Busy) {
+#endif //ENABLE_SD_CARD
                     vTaskEnterCritical(&myMutex);
                     client_buffer[client].write(data);
                     vTaskExitCritical(&myMutex);
+#if defined(ENABLE_SD_CARD)
                 } else {
                     if (data == '\r' || data == '\n') {
                         grbl_sendf(client, "error %d\r\n", Error::AnotherInterfaceBusy);
                         grbl_msg_sendf(client, MsgLevel::Info, "SD card job running");
                     }
                 }
+#endif //ENABLE_SD_CARD
             }
         }  // if something available
         WebUI::COMMANDS::handle();

Grbl_Esp32/src/WebUI/WebServer.cpp

@@ -480,10 +480,11 @@ namespace WebUI {
                 }
             }
             if (silent || !espresponse->anyOutput()) {
-                _webserver->send(err != Error::Ok ? 401 : 200, "text/plain", answer);
+                _webserver->send(err != Error::Ok ? 500 : 200, "text/plain", answer);
             } else {
                 espresponse->flush();
             }
+            if(espresponse) delete(espresponse);
         } else {  //execute GCODE
             if (auth_level == AuthenticationLevel::LEVEL_GUEST) {
                 _webserver->send(401, "text/plain", "Authentication failed!\n");
@@ -491,7 +492,7 @@ namespace WebUI {
             }
             //Instead of send several commands one by one by web  / send full set and split here
             String      scmd;
-            const char* res    = "";
+            bool hasError =false;
             uint8_t     sindex = 0;
             // TODO Settings - this is very inefficient.  get_Splited_Value() is O(n^2)
             // when it could easily be O(n).  Also, it would be just as easy to push
@@ -512,10 +513,10 @@ namespace WebUI {
                     scmd += "\n";
                 }
                 if (!Serial2Socket.push(scmd.c_str())) {
-                    res = "Error";
+                    hasError = true;
                 }
             }
-            _webserver->send(200, "text/plain", res);
+            _webserver->send(200, "text/plain", hasError?"Error":"");
         }
     }
 

Grbl_Esp32/src/WebUI/WebSettings.cpp

@@ -302,11 +302,11 @@ namespace WebUI {
         }
         if (!SPIFFS.exists(path)) {
             webPrintln("Error: No such file!");
-            return Error::SdFileNotFound;
+            return Error::FsFileNotFound;
         }
         File currentfile = SPIFFS.open(path, FILE_READ);
         if (!currentfile) {  //if file open success
-            return Error::SdFailedOpenFile;
+            return Error::FsFailedOpenFile;
         }
         //until no line in file
         Error   err;
@@ -338,11 +338,11 @@ namespace WebUI {
         }
         if (!SPIFFS.exists(path)) {
             webPrintln("Error: No such file!");
-            return Error::SdFileNotFound;
+            return Error::FsFileNotFound;
         }
         File currentfile = SPIFFS.open(path, FILE_READ);
         if (!currentfile) {
-            return Error::SdFailedOpenFile;
+            return Error::FsFailedOpenFile;
         }
         while (currentfile.available()) {
             // String currentline = currentfile.readStringUntil('\n');
@@ -686,16 +686,16 @@ namespace WebUI {
         if (state != SDState::Idle) {
             if (state == SDState::NotPresent) {
                 webPrintln("No SD Card");
-                return Error::SdFailedMount;
+                return Error::FsFailedMount;
             } else {
                 webPrintln("SD Card Busy");
-                return Error::SdFailedBusy;
+                return Error::FsFailedBusy;
             }
         }
         if (!openFile(SD, path.c_str())) {
-            report_status_message(Error::SdFailedRead, (espresponse) ? espresponse->client() : CLIENT_ALL);
+            report_status_message(Error::FsFailedRead, (espresponse) ? espresponse->client() : CLIENT_ALL);
             webPrintln("");
-            return Error::SdFailedOpenFile;
+            return Error::FsFailedOpenFile;
         }
         return Error::Ok;
     }
@@ -764,18 +764,18 @@ namespace WebUI {
         File file2del = SD.open(path);
         if (!file2del) {
             webPrintln("Cannot stat file!");
-            return Error::SdFileNotFound;
+            return Error::FsFileNotFound;
         }
         if (file2del.isDirectory()) {
             if (!SD.rmdir(path)) {
                 webPrintln("Cannot delete directory! Is directory empty?");
-                return Error::SdFailedDelDir;
+                return Error::FsFailedDelDir;
             }
             webPrintln("Directory deleted.");
         } else {
             if (!SD.remove(path)) {
                 webPrintln("Cannot delete file!");
-                return Error::SdFailedDelFile;
+                return Error::FsFailedDelFile;
             }
             webPrintln("File deleted.");
         }
@@ -788,10 +788,10 @@ namespace WebUI {
         if (state != SDState::Idle) {
             if (state == SDState::NotPresent) {
                 webPrintln("No SD Card");
-                return Error::SdFailedMount;
+                return Error::FsFailedMount;
             } else {
                 webPrintln("SD Card Busy");
-                return Error::SdFailedBusy;
+                return Error::FsFailedBusy;
             }
         }
         webPrintln("");
@@ -806,9 +806,35 @@ namespace WebUI {
     }
 #endif
 
+    void listDirLocalFS(fs::FS& fs, const char* dirname, uint8_t levels, uint8_t client) {
+        //char temp_filename[128]; // to help filter by extension	TODO: 128 needs a definition based on something
+        File root = fs.open(dirname);
+        if (!root) {
+            //FIXME: need proper error for FS and not usd sd one
+            report_status_message(Error::FsFailedOpenDir, client);
+            return;
+        }
+        if (!root.isDirectory()) {
+            //FIXME: need proper error for FS and not usd sd one
+            report_status_message(Error::FsDirNotFound, client);
+            return;
+        }
+        File file = root.openNextFile();
+        while (file) {
+            if (file.isDirectory()) {
+                if (levels) {
+                    listDirLocalFS(fs, file.name(), levels - 1, client);
+                }
+            } else {
+                grbl_sendf(CLIENT_ALL, "[FILE:%s|SIZE:%d]\r\n", file.name(), file.size());
+            }
+            file = root.openNextFile();
+        }
+    }
+
     static Error listLocalFiles(char* parameter, AuthenticationLevel auth_level) {  // No ESP command
         webPrintln("");
-        listDir(SPIFFS, "/", 10, espresponse->client());
+        listDirLocalFS(SPIFFS, "/", 10, espresponse->client());
         String ssd = "[Local FS Free:" + ESPResponseStream::formatBytes(SPIFFS.totalBytes() - SPIFFS.usedBytes());
         ssd += " Used:" + ESPResponseStream::formatBytes(SPIFFS.usedBytes());
         ssd += " Total:" + ESPResponseStream::formatBytes(SPIFFS.totalBytes());