From c3318fe3dd38d26d24fbd9a6eac0430d505af8a6 Mon Sep 17 00:00:00 2001 From: Mitch Bradley Date: Wed, 30 Dec 2020 10:08:49 -1000 Subject: [PATCH] Fix SD/List repetition error (#727) * Fix SD/List repetition error The one line change that actually fixes it is Serial.cpp line 162, where the SD state is compared to "not busy" instead of "is idle", thus also handling the "not present" case. In the process, I converted the "const int SDCARD_ ..." to an SDState enum class, thus proving type safety and eliminating yet another untyped uint8_t . * Updates after testing Co-authored-by: bdring --- Grbl_Esp32/src/Grbl.h | 2 +- Grbl_Esp32/src/Machines/template.h | 233 --------------------------- Grbl_Esp32/src/MotionControl.cpp | 2 +- Grbl_Esp32/src/Report.cpp | 6 +- Grbl_Esp32/src/SDCard.cpp | 20 +-- Grbl_Esp32/src/SDCard.h | 17 +- Grbl_Esp32/src/Serial.cpp | 2 +- Grbl_Esp32/src/WebUI/WebServer.cpp | 23 ++- Grbl_Esp32/src/WebUI/WebSettings.cpp | 24 +-- 9 files changed, 49 insertions(+), 280 deletions(-) delete mode 100644 Grbl_Esp32/src/Machines/template.h diff --git a/Grbl_Esp32/src/Grbl.h b/Grbl_Esp32/src/Grbl.h index 93c3f1e6..4cccc59a 100644 --- a/Grbl_Esp32/src/Grbl.h +++ b/Grbl_Esp32/src/Grbl.h @@ -23,7 +23,7 @@ // Grbl versioning system const char* const GRBL_VERSION = "1.3a"; -const char* const GRBL_VERSION_BUILD = "20201212"; +const char* const GRBL_VERSION_BUILD = "20201230"; //#include #include diff --git a/Grbl_Esp32/src/Machines/template.h b/Grbl_Esp32/src/Machines/template.h deleted file mode 100644 index 7672d31f..00000000 --- a/Grbl_Esp32/src/Machines/template.h +++ /dev/null @@ -1,233 +0,0 @@ -#pragma once -// clang-format off - -/* - template.h - Part of Grbl_ESP32 - - Template for a machine configuration file. - - 2020 - Mitch Bradley - - Grbl_ESP32 is free software: you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation, either version 3 of the License, or - (at your option) any later version. - - Grbl is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with Grbl_ESP32. If not, see . -*/ - -// This contains a long list of things that might possibly be -// configured. Most machines - especially simple cartesian machines -// that use stepper motors - will only need to define a few of the -// options herein, often just the pin assignments. - -// Pin assignments depend on how the ESP32 is connected to -// the external machine. Typically the ESP32 module plugs into -// an adapter board that wires specific ESP32 GPIO pins to -// other connectors on the board, such as Pololu sockets for -// stepper drivers or connectors for external drivers, limit -// pins, spindle control, etc. This file describes how those -// GPIO pins are wired to those other connectors. - -// Some machines might choose to use an adapter board in a -// non-standard way, for example a 3-axis board might have axes -// labeled XYZ, but the machine might have only 2 axes one of which is -// driven by two ganged motors. In that case, you would need -// a custom version of this file that assigns the pins differently -// from the adapter board labels. - -// In addition to pin assignments, many other aspects of Grbl -// can be configured, such as spindle speeds, special motor -// types like servos and unipolars, homing order, default values -// for $$ settings, etc. A detailed list of such options is -// given below. - -// Furthermore, it is possible to implement special complex -// behavior in custom C++ code, for non-Cartesian machines, -// unusual homing cycles, etc. See the Special Features section -// below for additional instructions. - -// === Machine Name -// Change TEMPLATE to some name of your own choosing. That name -// will be shown in a Grbl startup message to identify your -// configuration. - -#define MACHINE_NAME "TEMPLATE" - -// If your machine requires custom code as described below in -// Special Features, you must copy Custom/custom_code_template.cpp -// to a new name like Custom/my_custom_code.cpp, implement the -// functions therein, and enable its use by defining: -// #define CUSTOM_CODE_FILENAME "Custom/my_custom_code.cpp" - -// === Number of axes - -// You can set the number of axes that the machine supports -// by defining N_AXIS. If you do not define it, 3 will be -// used. The value must be at least 3, even if your machine -// has fewer axes. -// #define N_AXIS 4 - - -// == Pin Assignments - -// Step and direction pins; these must be output-capable pins, -// specifically ESP32 GPIO numbers 0..31 -// #define X_STEP_PIN GPIO_NUM_12 -// #define X_DIRECTION_PIN GPIO_NUM_14 -// #define Y_STEP_PIN GPIO_NUM_26 -// #define Y_DIRECTION_PIN GPIO_NUM_15 -// #define Z_STEP_PIN GPIO_NUM_27 -// #define Z_DIRECTION_PIN GPIO_NUM_33 - -// #define X_LIMIT_PIN GPIO_NUM_17 -// #define Y_LIMIT_PIN GPIO_NUM_4 -// #define Z_LIMIT_PIN GPIO_NUM_16 - -// Common enable for all steppers. If it is okay to leave -// your drivers enabled at all times, you can leave -// STEPPERS_DISABLE_PIN undefined and use the pin for something else. -// #define STEPPERS_DISABLE_PIN GPIO_NUM_13 - -// Pins for controlling various aspects of the machine. If your -// machine does not support one of these features, you can leave -// the corresponding pin undefined. - -// #define SPINDLE_OUTPUT_PIN GPIO_NUM_2 // labeled SpinPWM -// #define SPINDLE_ENABLE_PIN GPIO_NUM_22 // labeled SpinEnbl -// #define COOLANT_MIST_PIN GPIO_NUM_21 // labeled Mist -// #define COOLANT_FLOOD_PIN GPIO_NUM_25 // labeled Flood -// #define PROBE_PIN GPIO_NUM_32 // labeled Probe - -// Input pins for various functions. If the corresponding pin is not defined, -// the function will not be available. - -// CONTROL_SAFETY_DOOR_PIN shuts off the machine when a door is opened -// or some other unsafe condition exists. -// #define CONTROL_SAFETY_DOOR_PIN GPIO_NUM_35 // labeled Door, needs external pullup - -// RESET, FEED_HOLD, and CYCLE_START can control GCode execution at -// the push of a button. - -// #define CONTROL_RESET_PIN GPIO_NUM_34 // labeled Reset, needs external pullup -// #define CONTROL_FEED_HOLD_PIN GPIO_NUM_36 // labeled Hold, needs external pullup -// #define CONTROL_CYCLE_START_PIN GPIO_NUM_39 // labeled Start, needs external pullup - -// === Ganging -// If you need to use two motors on one axis, you can "gang" the motors by -// defining a second pin to control the other motor on the axis. For example: - -// #define Y2_STEP_PIN GPIO_NUM_27 /* labeled Z */ -// #define Y2_DIRECTION_PIN GPIO_NUM_33 /* labeled Z */ - -// === Servos -// To use a servo motor on an axis, do not define step and direction -// pins for that axis, but instead include a block like this: - -// #define SERVO_Z_PIN GPIO_NUM_22 - -// === Homing cycles -// Set them using $Homing/Cycle0= optionally up to $Homing/Cycle5= - -// === Default settings -// Grbl has many run-time settings that the user can changed by -// commands like $110=2000 . Their values are stored in non-volatile -// storage so they persist after the controller has been powered down. -// Those settings have default values that are used if the user -// has not altered them, or if the settings are explicitly reset -// to the default values wth $RST=$. -// -// The default values are established in defaults.h, but you -// can override any one of them by definining it here, for example: - -//#define DEFAULT_INVERT_LIMIT_PINS 1 -//#define DEFAULT_REPORT_INCHES 1 - -// === Control Pins - -// If some of the control pin switches are normally closed -// (the default is normally open), you can invert some of them -// with INVERT_CONTROL_PIN_MASK. The bits in the mask are -// Cycle Start, Feed Hold, Reset, Safety Door. To use a -// normally open switch on Reset, you would say -// #define INVERT_CONTROL_PIN_MASK B1101 - -// If your control pins do not have adequate hardware signal -// conditioning, you can define these to use software to -// reduce false triggering. -// #define ENABLE_CONTROL_SW_DEBOUNCE // Default disabled. Uncomment to enable. -// #define CONTROL_SW_DEBOUNCE_PERIOD 32 // in milliseconds default 32 microseconds - - -// Grbl_ESP32 use the ESP32's special RMT (IR remote control) hardware -// engine to achieve more precise high step rates than can be done -// in software. That feature is enabled by default, but there are -// some machines that might not want to use it, such as machines that -// do not use ordinary stepper motors. To turn it off, do this: -// #undef USE_RMT_STEPS - -// === Special Features -// Grbl_ESP32 can support non-Cartesian machines and some other -// scenarios that cannot be handled by choosing from a set of -// predefined selections. Instead they require machine-specific -// C++ code functions. There are callouts in the core code for -// such code, guarded by ifdefs that enable calling the individual -// functions. custom_code_template.cpp describes the functions -// that you can implement. The ifdef guards are described below: -// -// USE_CUSTOM_HOMING enables the user_defined_homing(uint8_t cycle_mask) function -// that can implement an arbitrary homing sequence. -// #define USE_CUSTOM_HOMING - -// USE_KINEMATICS enables the functions inverse_kinematics(), -// kinematics_pre_homing(), and kinematics_post_homing(), -// so non-Cartesian machines can be implemented. -// #define USE_KINEMATICS - -// USE_FWD_KINEMATICS enables the forward_kinematics() function -// that converts motor positions in non-Cartesian coordinate -// systems back to Cartesian form, for status reports. -//#define USE_FWD_KINEMATICS - -// USE_TOOL_CHANGE enables the user_tool_change() function -// that implements custom tool change procedures. -// #define USE_TOOL_CHANGE - -// Any one of MACRO_BUTTON_0_PIN, MACRO_BUTTON_1_PIN, and MACRO_BUTTON_2_PIN -// enables the user_defined_macro(number) function which -// implements custom behavior at the press of a button -// #define MACRO_BUTTON_0_PIN - -// USE_M30 enables the user_m30() function which implements -// custom behavior when a GCode programs stops at the end -// #define USE_M30 - -// USE_TRIAMINIC enables a suite of functions that are defined -// in grbl_triaminic.cpp, allowing the use of Triaminic stepper -// drivers that require software configuration at startup. -// There are several options that control the details of such -// drivers; inspect the code in grbl_triaminic.cpp to see them. -// #define USE_TRIAMINIC -// #define X_TRIAMINIC -// #define X_DRIVER_TMC2209 -// #define TRIAMINIC_DAISY_CHAIN - -// USE_MACHINE_TRINAMIC_INIT enables the machine_triaminic_setup() -// function that replaces the normal setup of Triaminic drivers. -// It could, for, example, setup StallGuard or other special modes. -// #define USE_MACHINE_TRINAMIC_INIT - - -// === Grbl behavior options -// There are quite a few options that control aspects of Grbl that -// are not specific to particular machines. They are listed and -// described in config.h after it includes the file machine.h. -// Normally you would not need to change them, but if you do, -// it will be necessary to make the change in config.h diff --git a/Grbl_Esp32/src/MotionControl.cpp b/Grbl_Esp32/src/MotionControl.cpp index 74b53d26..56be3e83 100644 --- a/Grbl_Esp32/src/MotionControl.cpp +++ b/Grbl_Esp32/src/MotionControl.cpp @@ -509,7 +509,7 @@ void mc_reset() { sys_pwm_control(0xFF, 0, false); #ifdef ENABLE_SD_CARD // do we need to stop a running SD job? - if (get_sd_state(false) == SDCARD_BUSY_PRINTING) { + if (get_sd_state(false) == SDState::BusyPrinting) { //Report print stopped report_feedback_message(Message::SdFileQuit); closeFile(); diff --git a/Grbl_Esp32/src/Report.cpp b/Grbl_Esp32/src/Report.cpp index 33c53c11..67d4fddd 100644 --- a/Grbl_Esp32/src/Report.cpp +++ b/Grbl_Esp32/src/Report.cpp @@ -222,7 +222,7 @@ void report_status_message(Error status_code, uint8_t client) { switch (status_code) { case Error::Ok: // Error::Ok #ifdef ENABLE_SD_CARD - if (get_sd_state(false) == SDCARD_BUSY_PRINTING) { + if (get_sd_state(false) == SDState::BusyPrinting) { SD_ready_next = true; // flag so system_execute_line() will send the next line } else { grbl_send(client, "ok\r\n"); @@ -234,7 +234,7 @@ void report_status_message(Error status_code, uint8_t client) { default: #ifdef ENABLE_SD_CARD // do we need to stop a running SD job? - if (get_sd_state(false) == SDCARD_BUSY_PRINTING) { + if (get_sd_state(false) == SDState::BusyPrinting) { 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, sd_get_current_line_number()); @@ -837,7 +837,7 @@ void report_realtime_status(uint8_t client) { } #endif #ifdef ENABLE_SD_CARD - if (get_sd_state(false) == SDCARD_BUSY_PRINTING) { + if (get_sd_state(false) == SDState::BusyPrinting) { sprintf(temp, "|SD:%4.2f,", sd_report_perc_complete()); strcat(status, temp); sd_get_current_filename(temp); diff --git a/Grbl_Esp32/src/SDCard.cpp b/Grbl_Esp32/src/SDCard.cpp index 2a6fab21..32dd95ef 100644 --- a/Grbl_Esp32/src/SDCard.cpp +++ b/Grbl_Esp32/src/SDCard.cpp @@ -67,7 +67,7 @@ boolean openFile(fs::FS& fs, const char* path) { //report_status_message(Error::SdFailedRead, CLIENT_SERIAL); return false; } - set_sd_state(SDCARD_BUSY_PRINTING); + set_sd_state(SDState::BusyPrinting); SD_ready_next = false; // this will get set to true when Grbl issues "ok" message sd_current_line_number = 0; return true; @@ -77,7 +77,7 @@ boolean closeFile() { if (!myFile) { return false; } - set_sd_state(SDCARD_IDLE); + set_sd_state(SDState::Idle); SD_ready_next = false; sd_current_line_number = 0; myFile.close(); @@ -125,19 +125,19 @@ uint32_t sd_get_current_line_number() { return sd_current_line_number; } -uint8_t sd_state = SDCARD_IDLE; +SDState sd_state = SDState::Idle; -uint8_t get_sd_state(bool refresh) { +SDState get_sd_state(bool refresh) { if (SDCARD_DET_PIN != UNDEFINED_PIN) { if (digitalRead(SDCARD_DET_PIN) != SDCARD_DET_VAL) { - sd_state = SDCARD_NOT_PRESENT; + sd_state = SDState::NotPresent; return sd_state; //no need to go further if SD detect is not correct } } //if busy doing something return state - if (!((sd_state == SDCARD_NOT_PRESENT) || (sd_state == SDCARD_IDLE))) { + if (!((sd_state == SDState::NotPresent) || (sd_state == SDState::Idle))) { return sd_state; } if (!refresh) { @@ -146,19 +146,19 @@ uint8_t get_sd_state(bool refresh) { //SD is idle or not detected, let see if still the case SD.end(); - sd_state = SDCARD_NOT_PRESENT; + sd_state = SDState::NotPresent; //using default value for speed ? should be parameter //refresh content if card was removed if (SD.begin((GRBL_SPI_SS == -1) ? SS : GRBL_SPI_SS, SPI, GRBL_SPI_FREQ, "/sd", 2)) { if (SD.cardSize() > 0) { - sd_state = SDCARD_IDLE; + sd_state = SDState::Idle; } } return sd_state; } -uint8_t set_sd_state(uint8_t flag) { - sd_state = flag; +SDState set_sd_state(SDState state) { + sd_state = state; return sd_state; } diff --git a/Grbl_Esp32/src/SDCard.h b/Grbl_Esp32/src/SDCard.h index 298fbdec..a680e0a4 100644 --- a/Grbl_Esp32/src/SDCard.h +++ b/Grbl_Esp32/src/SDCard.h @@ -23,19 +23,22 @@ //#define SDCARD_DET_PIN -1 const int SDCARD_DET_VAL = 0; // for now, CD is close to ground -const int SDCARD_IDLE = 0; -const int SDCARD_NOT_PRESENT = 1; -const int SDCARD_BUSY_PRINTING = 2; -const int SDCARD_BUSY_UPLOADING = 4; -const int SDCARD_BUSY_PARSING = 8; +enum class SDState : uint8_t { + Idle = 0, + NotPresent = 1, + Busy = 2, + BusyPrinting = 2, + BusyUploading = 3, + BusyParsing = 4, +}; extern bool SD_ready_next; // Grbl has processed a line and is waiting for another extern uint8_t SD_client; extern WebUI::AuthenticationLevel SD_auth_level; //bool sd_mount(); -uint8_t get_sd_state(bool refresh); -uint8_t set_sd_state(uint8_t flag); +SDState get_sd_state(bool refresh); +SDState set_sd_state(SDState state); void listDir(fs::FS& fs, const char* dirname, uint8_t levels, uint8_t client); boolean openFile(fs::FS& fs, const char* path); boolean closeFile(); diff --git a/Grbl_Esp32/src/Serial.cpp b/Grbl_Esp32/src/Serial.cpp index 5e5e2549..9a4b9e4e 100644 --- a/Grbl_Esp32/src/Serial.cpp +++ b/Grbl_Esp32/src/Serial.cpp @@ -159,7 +159,7 @@ void serialCheckTask(void* pvParameters) { if (is_realtime_command(data)) { execute_realtime_command(static_cast(data), client); } else { - if (get_sd_state(false) == SDCARD_IDLE) { + if (get_sd_state(false) < SDState::Busy) { vTaskEnterCritical(&myMutex); client_buffer[client].write(data); vTaskExitCritical(&myMutex); diff --git a/Grbl_Esp32/src/WebUI/WebServer.cpp b/Grbl_Esp32/src/WebUI/WebServer.cpp index d58a8b7c..97b2ca34 100644 --- a/Grbl_Esp32/src/WebUI/WebServer.cpp +++ b/Grbl_Esp32/src/WebUI/WebServer.cpp @@ -1221,16 +1221,15 @@ namespace WebUI { bool list_files = true; uint64_t totalspace = 0; uint64_t usedspace = 0; - int8_t state = get_sd_state(true); - if (state != SDCARD_IDLE) { + SDState state = get_sd_state(true); + if (state != SDState::Idle) { String status = "{\"status\":\""; - if(state == SDCARD_NOT_PRESENT)status+="No SD Card\"}"; - else status+="Busy\"}"; + status += state == SDState::NotPresent ? "No SD Card\"}" : "Busy\"}"; _webserver->sendHeader("Cache-Control", "no-cache"); _webserver->send(200, "application/json", status); return; } - set_sd_state(SDCARD_BUSY_PARSING); + set_sd_state(SDState::BusyParsing); //get current path if (_webserver->hasArg("path")) { @@ -1400,7 +1399,7 @@ namespace WebUI { _webserver->sendHeader("Cache-Control", "no-cache"); _webserver->send(200, "application/json", jsonfile); _upload_status = UploadStatusType::NONE; - set_sd_state(SDCARD_IDLE); + set_sd_state(SDState::Idle); SD.end(); } @@ -1427,13 +1426,13 @@ namespace WebUI { filename = "/" + upload.filename; } //check if SD Card is available - if (get_sd_state(true) != SDCARD_IDLE) { + if (get_sd_state(true) != SDState::Idle) { _upload_status = UploadStatusType::FAILED; grbl_send(CLIENT_ALL, "[MSG:Upload cancelled]\r\n"); pushError(ESP_ERROR_UPLOAD_CANCELLED, "Upload cancelled"); } else { - set_sd_state(SDCARD_BUSY_UPLOADING); + set_sd_state(SDState::BusyUploading); //delete file on SD Card if already present if (SD.exists(filename)) { SD.remove(filename); @@ -1468,7 +1467,7 @@ namespace WebUI { //************** } else if (upload.status == UPLOAD_FILE_WRITE) { vTaskDelay(1 / portTICK_RATE_MS); - if (sdUploadFile && (_upload_status == UploadStatusType::ONGOING) && (get_sd_state(false) == SDCARD_BUSY_UPLOADING)) { + if (sdUploadFile && (_upload_status == UploadStatusType::ONGOING) && (get_sd_state(false) == SDState::BusyUploading)) { //no error write post data if (upload.currentSize != sdUploadFile.write(upload.buf, upload.currentSize)) { _upload_status = UploadStatusType::FAILED; @@ -1506,7 +1505,7 @@ namespace WebUI { } if (_upload_status == UploadStatusType::ONGOING) { _upload_status = UploadStatusType::SUCCESSFUL; - set_sd_state(SDCARD_IDLE); + set_sd_state(SDState::Idle); } else { _upload_status = UploadStatusType::FAILED; pushError(ESP_ERROR_UPLOAD, "Upload error"); @@ -1514,7 +1513,7 @@ namespace WebUI { } else { //Upload cancelled _upload_status = UploadStatusType::FAILED; - set_sd_state(SDCARD_IDLE); + set_sd_state(SDState::Idle); grbl_send(CLIENT_ALL, "[MSG:Upload failed]\r\n"); if (sdUploadFile) { sdUploadFile.close(); @@ -1532,7 +1531,7 @@ namespace WebUI { if (SD.exists(filename)) { SD.remove(filename); } - set_sd_state(SDCARD_IDLE); + set_sd_state(SDState::Idle); } COMMANDS::wait(0); } diff --git a/Grbl_Esp32/src/WebUI/WebSettings.cpp b/Grbl_Esp32/src/WebUI/WebSettings.cpp index cec830b9..7058b31d 100644 --- a/Grbl_Esp32/src/WebUI/WebSettings.cpp +++ b/Grbl_Esp32/src/WebUI/WebSettings.cpp @@ -682,9 +682,9 @@ namespace WebUI { if (path[0] != '/') { path = "/" + path; } - int8_t state = get_sd_state(true); - if (state != SDCARD_IDLE) { - if (state == SDCARD_NOT_PRESENT) { + SDState state = get_sd_state(true); + if (state != SDState::Idle) { + if (state == SDState::NotPresent) { webPrintln("No SD Card"); return Error::SdFailedMount; } else { @@ -752,9 +752,9 @@ namespace WebUI { webPrintln("Missing file name!"); return Error::InvalidValue; } - int8_t state = get_sd_state(true); - if (state != SDCARD_IDLE) { - webPrintln((state == SDCARD_NOT_PRESENT) ? "No SD card" : "Busy"); + SDState state = get_sd_state(true); + if (state != SDState::Idle) { + webPrintln((state == SDState::NotPresent) ? "No SD card" : "Busy"); return Error::Ok; } String path = parameter; @@ -784,9 +784,9 @@ namespace WebUI { } static Error listSDFiles(char* parameter, AuthenticationLevel auth_level) { // ESP210 - int8_t state = get_sd_state(true); - if (state != SDCARD_IDLE) { - if (state == SDCARD_NOT_PRESENT) { + SDState state = get_sd_state(true); + if (state != SDState::Idle) { + if (state == SDState::NotPresent) { webPrintln("No SD Card"); return Error::SdFailedMount; } else { @@ -857,10 +857,10 @@ namespace WebUI { const char* resp = "No SD card"; #ifdef ENABLE_SD_CARD switch (get_sd_state(true)) { - case SDCARD_IDLE: + case SDState::Idle: resp = "SD card detected"; break; - case SDCARD_NOT_PRESENT: + case SDState::NotPresent: resp = "No SD card"; break; default: @@ -868,7 +868,7 @@ namespace WebUI { } #else resp = "SD card not enabled"; -#endif +#endif webPrintln(resp); return Error::Ok; }