From 897b93fe9a8f8ec00e9354ebf6fcca4e6a10e2cd Mon Sep 17 00:00:00 2001 From: Mitch Bradley Date: Sun, 4 Jul 2021 14:43:52 -1000 Subject: [PATCH] Better error handling in SDCard --- Grbl_Esp32/src/Error.cpp | 4 +- Grbl_Esp32/src/Error.h | 1 + Grbl_Esp32/src/Machine/SPIBus.cpp | 23 ++++++-- Grbl_Esp32/src/Protocol.cpp | 26 +++++---- Grbl_Esp32/src/Report.cpp | 83 +++++++++++++++++----------- Grbl_Esp32/src/SDCard.cpp | 48 ++++++---------- Grbl_Esp32/src/SDCard.h | 13 +++-- Grbl_Esp32/src/WebUI/WebSettings.cpp | 20 ++++--- 8 files changed, 125 insertions(+), 93 deletions(-) diff --git a/Grbl_Esp32/src/Error.cpp b/Grbl_Esp32/src/Error.cpp index 7720947d..92abbf9e 100644 --- a/Grbl_Esp32/src/Error.cpp +++ b/Grbl_Esp32/src/Error.cpp @@ -62,7 +62,7 @@ std::map ErrorNames = { { Error::GcodeMaxValueExceeded, "Gcode max value exceeded" }, { Error::PParamMaxExceeded, "P param max exceeded" }, { Error::FsFailedMount, "Failed to mount device" }, - { Error::FsFailedRead, "Failed to read" }, + { Error::FsFailedRead, "Read failed" }, { Error::FsFailedOpenDir, "Failed to open directory" }, { Error::FsDirNotFound, "Directory not found" }, { Error::FsFileEmpty, "File empty" }, @@ -79,6 +79,8 @@ std::map ErrorNames = { { Error::NvsSetFailed, "Failed to store setting" }, { Error::NvsGetStatsFailed, "Failed to get setting status" }, { Error::AuthenticationFailed, "Authentication failed!" }, + { Error::Eol, "End of line" }, + { Error::Eof, "End of file" }, { Error::AnotherInterfaceBusy, "Another interface is busy" }, { Error::BadPinSpecification, "Bad Pin Specification" }, { Error::JogCancelled, "Jog Cancelled" }, diff --git a/Grbl_Esp32/src/Error.h b/Grbl_Esp32/src/Error.h index 9f4e52d1..b24f3d5e 100644 --- a/Grbl_Esp32/src/Error.h +++ b/Grbl_Esp32/src/Error.h @@ -85,6 +85,7 @@ enum class Error : uint8_t { NvsGetStatsFailed = 101, AuthenticationFailed = 110, Eol = 111, + Eof = 112, // Not necessarily an error AnotherInterfaceBusy = 120, JogCancelled = 130, BadPinSpecification = 150, diff --git a/Grbl_Esp32/src/Machine/SPIBus.cpp b/Grbl_Esp32/src/Machine/SPIBus.cpp index d57393d0..8c7b4bc1 100644 --- a/Grbl_Esp32/src/Machine/SPIBus.cpp +++ b/Grbl_Esp32/src/Machine/SPIBus.cpp @@ -17,6 +17,7 @@ */ #include "SPIBus.h" +#include "../Report.h" #include @@ -32,11 +33,19 @@ namespace Machine { void SPIBus::init() { if (_cs.defined()) { // validation ensures the rest is also defined. + info_serial( + "SPI SCK:%s MOSI:%s MISO:%s CS:%s", _sck.name().c_str(), _mosi.name().c_str(), _miso.name().c_str(), _cs.name().c_str()); + + _cs.setAttr(Pin::Attr::Output); + auto csPin = _cs.getNative(Pin::Capabilities::Output | Pin::Capabilities::Native); auto mosiPin = _mosi.getNative(Pin::Capabilities::Output | Pin::Capabilities::Native); auto sckPin = _sck.getNative(Pin::Capabilities::Output | Pin::Capabilities::Native); auto misoPin = _miso.getNative(Pin::Capabilities::Input | Pin::Capabilities::Native); + // Start the SPI bus with the pins defined here. Once it has been started, + // those pins "stick" and subsequent attempts to restart it with defaults + // for the miso, mosi, and sck pins are ignored SPI.begin(sckPin, misoPin, mosiPin, csPin); } } @@ -48,13 +57,19 @@ namespace Machine { handler.item("sck", _sck); } + // XXX it would be nice to have some way to turn off SPI entirely void SPIBus::afterParse() { - if (_cs.undefined() && _miso.undefined() && _mosi.undefined() && _sck.undefined()) { - // Default SPI miso, mosi, sck, cs pins to the "standard" gpios 19, 23, 18, 5 + if (_cs.undefined()) { + _cs = Pin::create("gpio.5"); + } + if (_miso.undefined()) { _miso = Pin::create("gpio.19"); + } + if (_mosi.undefined()) { _mosi = Pin::create("gpio.23"); - _sck = Pin::create("gpio.18"); - _cs = Pin::create("gpio.5"); + } + if (_sck.undefined()) { + _sck = Pin::create("gpio.18"); } } } diff --git a/Grbl_Esp32/src/Protocol.cpp b/Grbl_Esp32/src/Protocol.cpp index 31bdd5b5..81c33020 100644 --- a/Grbl_Esp32/src/Protocol.cpp +++ b/Grbl_Esp32/src/Protocol.cpp @@ -167,16 +167,22 @@ void protocol_main_loop() { int c; for (;;) { auto sdcard = config->_sdCard; - if (sdcard->_ready_next) { - char fileLine[255]; - if (sdcard->readFileLine(fileLine, 255)) { - sdcard->_ready_next = false; - report_status_message(execute_line(fileLine, sdcard->_client, sdcard->_auth_level), sdcard->_client); - } else { - char temp[50]; - sdcard->get_current_filename(temp); - grbl_notifyf("SD print done", "%s print is successful", temp); - sdcard->closeFile(); // close file and clear SD ready/running flags + // _readyNext indicates that input is coming from a file and + // the GCode system is ready for another line. + if (sdcard->_readyNext) { + char fileLine[255]; + Error res; + switch (res = sdcard->readFileLine(fileLine, 255)) { + case Error::Ok: + sdcard->_readyNext = false; +#ifdef DEBUG_REPORT_ECHO_RAW_LINE_RECEIVED + report_echo_line_received(fileLine, CLIENT_SERIAL); +#endif + report_status_message(execute_line(fileLine, sdcard->_client, sdcard->_auth_level), sdcard->_client); + break; + default: + report_status_message(res, sdcard->_client); + break; } } // Receive one line of incoming serial data, as the data becomes available. diff --git a/Grbl_Esp32/src/Report.cpp b/Grbl_Esp32/src/Report.cpp index 5d2e186e..19c5f971 100644 --- a/Grbl_Esp32/src/Report.cpp +++ b/Grbl_Esp32/src/Report.cpp @@ -241,38 +241,56 @@ static String report_util_axis_values(const float* axis_value) { // from a critical error, such as a triggered hard limit. Interface should always monitor for these // responses. void report_status_message(Error status_code, uint8_t client) { - auto sdCard = config->_sdCard; - switch (status_code) { - case Error::Ok: // Error::Ok - if (sdCard->get_state(false) == SDCard::State::BusyPrinting) { - sdCard->_ready_next = true; // flag so system_execute_line() will send the next line - } else { - grbl_send(client, "ok\r\n"); - } - break; - default: - // do we need to stop a running SD job? - if (sdCard->get_state(false) == SDCard::State::BusyPrinting) { + auto sdcard = config->_sdCard; + if (sdcard->get_state(false) == SDCard::State::BusyPrinting) { + // When running from SD, the GCode is not coming from a sender, so we are not + // using the Grbl send/response/error protocol. We use _readyNext instead of + // "ok" to indicate readiness for another line, and we report verbose error + // messages with [MSG: ...] encapsulation + switch (status_code) { + case Error::Ok: + sdcard->_readyNext = true; // flag so system_execute_line() will send the next line + break; + case Error::Eof: + // XXX we really should wait for the machine to return to idle before + // we issue this message. What Eof really means is that all the lines + // in the file were sent to Grbl. Some could still be running. + grbl_notifyf("SD print done", "%s print succeeded", sdcard->filename()); + info_client(sdcard->_client, "%s print succeeded", sdcard->filename()); + sdcard->closeFile(); + break; + default: + info_client(sdcard->_client, + "Error:%d (%s) in %s at line %d", + status_code, + errorString(status_code), + sdcard->filename(), + sdcard->lineNumber()); if (status_code == Error::GcodeUnsupportedCommand) { - grbl_sendf(client, "error:%d\r\n", status_code); // most senders seem to tolerate this error and keep on going - grbl_sendf(CLIENT_ALL, "error:%d in SD file at line %d\r\n", status_code, sdCard->get_current_line_number()); - // don't close file - sdCard->_ready_next = true; // flag so system_execute_line() will send the next line + // Do not stop on unsupported commands because most senders do not + sdcard->_readyNext = true; } else { - grbl_notifyf("SD print error", "Error:%d during SD file at line: %d", status_code, sdCard->get_current_line_number()); - grbl_sendf(CLIENT_ALL, "error:%d in SD file at line %d\r\n", status_code, sdCard->get_current_line_number()); - sdCard->closeFile(); + grbl_notifyf("SD print error", "Error:%d in %s at line: %d", status_code, sdcard->filename(), sdcard->lineNumber()); + sdcard->closeFile(); } - return; - } - // With verbose errors, the message text is displayed instead of the number. - // Grbl 0.9 used to display the text, while Grbl 1.1 switched to the number. - // Many senders support both formats. - if (config->_verboseErrors) { - grbl_sendf(client, "error: %s\r\n", errorString(status_code)); - } else { - grbl_sendf(client, "error:%d\r\n", static_cast(status_code)); - } + } + } else { + // Input is coming from a sender so use the classic Grbl line protocol + switch (status_code) { + case Error::Ok: // Error::Ok + grbl_send(client, "ok\r\n"); + break; + default: + // With verbose errors, the message text is displayed instead of the number. + // Grbl 0.9 used to display the text, while Grbl 1.1 switched to the number. + // Many senders support both formats. + if (config->_verboseErrors) { + grbl_sendf(client, "error: %s\r\n", errorString(status_code)); + } else { + grbl_sendf(client, "error:%d\r\n", static_cast(status_code)); + } + break; + } } } @@ -306,8 +324,8 @@ std::map MessageText = { // is installed, the message number codes are less than zero. void report_feedback_message(Message message) { // ok to send to all clients if (message == Message::SdFileQuit) { - grbl_notifyf("SD print canceled", "Reset during SD file at line: %d", config->_sdCard->get_current_line_number()); - info_serial("Reset during SD file at line: %d", config->_sdCard->get_current_line_number()); + grbl_notifyf("SD print canceled", "Reset during SD file at line: %d", config->_sdCard->lineNumber()); + info_serial("Reset during SD file at line: %d", config->_sdCard->lineNumber()); } else { auto it = MessageText.find(message); @@ -757,8 +775,7 @@ void report_realtime_status(uint8_t client) { if (config->_sdCard->get_state(false) == SDCard::State::BusyPrinting) { sprintf(temp, "|SD:%4.2f,", config->_sdCard->report_perc_complete()); strcat(status, temp); - config->_sdCard->get_current_filename(temp); - strcat(status, temp); + strcat(status, config->_sdCard->filename()); } #ifdef DEBUG_STEPPER_ISR sprintf(temp, "|ISRs:%d", step_count); diff --git a/Grbl_Esp32/src/SDCard.cpp b/Grbl_Esp32/src/SDCard.cpp index 5882fdb7..ce15c690 100644 --- a/Grbl_Esp32/src/SDCard.cpp +++ b/Grbl_Esp32/src/SDCard.cpp @@ -38,16 +38,6 @@ SDCard::SDCard() : _pImpl(new FileWrap()), _current_line_number(0), _state(State::Idle), _readyNext(false), _client(CLIENT_SERIAL), _auth_level(WebUI::AuthenticationLevel::LEVEL_GUEST) {} -// attempt to mount the SD card -/*bool SDCard::mount() -{ - if(!SD.begin()) { - report_status_message(Error::FsFailedMount, _client); - return false; - } - return true; -}*/ - void SDCard::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); @@ -85,44 +75,44 @@ bool SDCard::openFile(fs::FS& fs, const char* path) { } bool SDCard::closeFile() { - if (!_pImpl->_file) { - return false; - } set_state(State::Idle); _readyNext = false; _current_line_number = 0; + if (!_pImpl->_file) { + return false; + } _pImpl->_file.close(); SD.end(); return true; } /* - read a line from the SD card - strip whitespace - strip comments per http://linuxcnc.org/docs/ja/html/gcode/overview.html#gcode:comments - make uppercase - return true if a line is + Read a line from the SD card + Returns true if a line was read, even if it was empty. + Returns false on EOF or error. Errors display a message. */ -bool SDCard::readFileLine(char* line, int maxlen) { +Error SDCard::readFileLine(char* line, int maxlen) { if (!_pImpl->_file) { - report_status_message(Error::FsFailedRead, _client); - return false; + return Error::FsFailedRead; } _current_line_number += 1; int len = 0; while (_pImpl->_file.available()) { if (len >= maxlen) { - return false; + return Error::LineLengthExceeded; + } + int c = _pImpl->_file.read(); + if (c < 0) { + return Error::FsFailedRead; } - char c = _pImpl->_file.read(); if (c == '\n') { break; } line[len++] = c; } line[len] = '\0'; - return len || _pImpl->_file.available(); + return len || _pImpl->_file.available() ? Error::Ok : Error::Eof; } // return a percentage complete 50.5 = 50.5% @@ -133,7 +123,7 @@ float SDCard::report_perc_complete() { return (float)_pImpl->_file.position() / (float)_pImpl->_file.size() * 100.0f; } -uint32_t SDCard::get_current_line_number() { +uint32_t SDCard::lineNumber() { return _current_line_number; } @@ -181,12 +171,8 @@ SDCard::State SDCard::set_state(SDCard::State state) { return _state; } -void SDCard::get_current_filename(char* name) { - if (_pImpl->_file) { - strcpy(name, _pImpl->_file.name()); - } else { - name[0] = 0; - } +const char* SDCard::filename() { + return _pImpl->_file ? _pImpl->_file.name() : ""; } void SDCard::init() { diff --git a/Grbl_Esp32/src/SDCard.h b/Grbl_Esp32/src/SDCard.h index b31cfe75..c91e201c 100644 --- a/Grbl_Esp32/src/SDCard.h +++ b/Grbl_Esp32/src/SDCard.h @@ -18,6 +18,7 @@ #include "Configuration/Configurable.h" #include "WebUI/Authentication.h" #include "Pin.h" +#include "Error.h" #include @@ -53,7 +54,8 @@ private: Pin _cardDetect; public: - bool _readyNext; + bool _readyNext; // Grbl has processed a line and is waiting for another + uint8_t _client; WebUI::AuthenticationLevel _auth_level; @@ -61,8 +63,6 @@ public: SDCard(const SDCard&) = delete; SDCard& operator=(const SDCard&) = delete; - bool _ready_next = false; // Grbl has processed a line and is waiting for another - //bool mount(); SDCard::State get_state(bool refresh); SDCard::State set_state(SDCard::State state); @@ -70,10 +70,11 @@ public: void listDir(fs::FS& fs, const char* dirname, uint8_t levels, uint8_t client); bool openFile(fs::FS& fs, const char* path); bool closeFile(); - bool readFileLine(char* line, int len); + Error readFileLine(char* line, int len); float report_perc_complete(); - uint32_t get_current_line_number(); - void get_current_filename(char* name); + uint32_t lineNumber(); + + const char* filename(); // Initializes pins. void init(); diff --git a/Grbl_Esp32/src/WebUI/WebSettings.cpp b/Grbl_Esp32/src/WebUI/WebSettings.cpp index 1742ddd3..49ac592c 100644 --- a/Grbl_Esp32/src/WebUI/WebSettings.cpp +++ b/Grbl_Esp32/src/WebUI/WebSettings.cpp @@ -663,10 +663,14 @@ namespace WebUI { return err; } config->_sdCard->_client = (espresponse) ? espresponse->client() : CLIENT_ALL; - char fileLine[255]; - while (config->_sdCard->readFileLine(fileLine, 255)) { + char fileLine[255]; + Error res; + while ((res = config->_sdCard->readFileLine(fileLine, 255)) == Error::Ok) { webPrintln(fileLine); } + if (res != Error::Eof) { + webPrintln(errorString(res)); + } webPrintln(""); config->_sdCard->closeFile(); return Error::Ok; @@ -687,19 +691,19 @@ namespace WebUI { } auto sdCard = config->_sdCard; - char fileLine[255]; - if (!sdCard->readFileLine(fileLine, 255)) { - //No need notification here it is just a macro - sdCard->closeFile(); + char fileLine[255]; + Error res = sdCard->readFileLine(fileLine, 255); + if (res != Error::Ok) { + report_status_message(res, sdCard->_client); + // report_status_message will close the file webPrintln(""); return Error::Ok; } sdCard->_client = (espresponse) ? espresponse->client() : CLIENT_ALL; sdCard->_auth_level = auth_level; - // execute the first line now; Protocol.cpp handles later ones when sdCard._ready_next + // execute the first line now; Protocol.cpp handles later ones when sdCard._readyNext report_status_message(execute_line(fileLine, sdCard->_client, sdCard->_auth_level), sdCard->_client); report_realtime_status(sdCard->_client); - webPrintln(""); return Error::Ok; }