From d24df1fda47a22827ae5987344f8899e5c9e3786 Mon Sep 17 00:00:00 2001 From: Luc <8822552+luc-github@users.noreply.github.com> Date: Fri, 5 Mar 2021 17:04:39 +0100 Subject: [PATCH] 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 ad49cf8cc1cb2064a8f7687cb4c354eba4bc8172. * Updated error code symbols and text - To be generic file system error Co-authored-by: Luc Co-authored-by: bdring --- Grbl_Esp32/src/Error.cpp | 22 ++++++------ Grbl_Esp32/src/Error.h | 20 +++++------ Grbl_Esp32/src/Grbl.h | 2 +- Grbl_Esp32/src/Report.cpp | 6 +++- Grbl_Esp32/src/SDCard.cpp | 15 ++++---- Grbl_Esp32/src/Serial.cpp | 4 +++ Grbl_Esp32/src/WebUI/WebServer.cpp | 9 ++--- Grbl_Esp32/src/WebUI/WebSettings.cpp | 54 ++++++++++++++++++++-------- 8 files changed, 85 insertions(+), 47 deletions(-) diff --git a/Grbl_Esp32/src/Error.cpp b/Grbl_Esp32/src/Error.cpp index 34dd1cd4..c18ef200 100644 --- a/Grbl_Esp32/src/Error.cpp +++ b/Grbl_Esp32/src/Error.cpp @@ -61,16 +61,16 @@ std::map 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::SdFailedRead, "SD failed read" }, - { Error::SdFailedOpenDir, "SD failed to open directory" }, - { Error::SdDirNotFound, "SD directory not found" }, - { Error::SdFileEmpty, "SD file empty" }, - { Error::SdFileNotFound, "SD file not found" }, - { Error::SdFailedOpenFile, "SD failed to open file" }, - { Error::SdFailedBusy, "SD is busy" }, - { Error::SdFailedDelDir, "SD failed to delete directory" }, - { Error::SdFailedDelFile, "SD failed to delete file" }, + { Error::FsFailedMount, "Failed to mount device" }, + { Error::FsFailedRead, "Failed to read" }, + { Error::FsFailedOpenDir, "Failed to open directory" }, + { Error::FsDirNotFound, "Directory not found" }, + { Error::FsFileEmpty, "File empty" }, + { Error::FsFileNotFound, "File not found" }, + { Error::FsFailedOpenFile, "Failed to open file" }, + { Error::FsFailedBusy, "Device is busy" }, + { Error::FsFailedDelDir, "Failed to delete directory" }, + { 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 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" }, }; diff --git a/Grbl_Esp32/src/Error.h b/Grbl_Esp32/src/Error.h index 80f40a39..14b443eb 100644 --- a/Grbl_Esp32/src/Error.h +++ b/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 - SdFailedRead = 61, // SD Failed to read file - SdFailedOpenDir = 62, // SD card failed to open directory - SdDirNotFound = 63, // SD Card directory not found - SdFileEmpty = 64, // SD Card directory not found - SdFileNotFound = 65, // SD Card file not found - SdFailedOpenFile = 66, // SD card failed to open file - SdFailedBusy = 67, // SD card is busy - SdFailedDelDir = 68, - SdFailedDelFile = 69, + FsFailedMount = 60, // SD Failed to mount + FsFailedRead = 61, // SD Failed to read file + FsFailedOpenDir = 62, // SD card failed to open directory + FsDirNotFound = 63, // SD Card directory not found + FsFileEmpty = 64, // SD Card directory not found + FsFileNotFound = 65, // SD Card file not found + FsFailedOpenFile = 66, // SD card failed to open file + FsFailedBusy = 67, // SD card is busy + FsFailedDelDir = 68, + FsFailedDelFile = 69, BtFailBegin = 70, // Bluetooth failed to start WifiFailBegin = 71, // WiFi failed to start NumberRange = 80, // Setting number range problem diff --git a/Grbl_Esp32/src/Grbl.h b/Grbl_Esp32/src/Grbl.h index b6d0c438..2172f344 100644 --- a/Grbl_Esp32/src/Grbl.h +++ b/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 #include diff --git a/Grbl_Esp32/src/Report.cpp b/Grbl_Esp32/src/Report.cpp index 892230e8..b4728154 100644 --- a/Grbl_Esp32/src/Report.cpp +++ b/Grbl_Esp32/src/Report.cpp @@ -291,10 +291,14 @@ std::map 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); diff --git a/Grbl_Esp32/src/SDCard.cpp b/Grbl_Esp32/src/SDCard.cpp index 32dd95ef..eadac19f 100644 --- a/Grbl_Esp32/src/SDCard.cpp +++ b/Grbl_Esp32/src/SDCard.cpp @@ -18,7 +18,9 @@ along with Grbl. If not, see . */ -#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 diff --git a/Grbl_Esp32/src/Serial.cpp b/Grbl_Esp32/src/Serial.cpp index 9a4b9e4e..c5d0220b 100644 --- a/Grbl_Esp32/src/Serial.cpp +++ b/Grbl_Esp32/src/Serial.cpp @@ -159,16 +159,20 @@ void serialCheckTask(void* pvParameters) { if (is_realtime_command(data)) { execute_realtime_command(static_cast(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(); diff --git a/Grbl_Esp32/src/WebUI/WebServer.cpp b/Grbl_Esp32/src/WebUI/WebServer.cpp index 311c980a..b4a1cef5 100644 --- a/Grbl_Esp32/src/WebUI/WebServer.cpp +++ b/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":""); } } diff --git a/Grbl_Esp32/src/WebUI/WebSettings.cpp b/Grbl_Esp32/src/WebUI/WebSettings.cpp index 84e2ec18..f2a312f2 100644 --- a/Grbl_Esp32/src/WebUI/WebSettings.cpp +++ b/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());