diff --git a/Grbl_Esp32/src/Configuration/AfterParse.cpp b/Grbl_Esp32/src/Configuration/AfterParse.cpp index 0bea6169..5d7871c4 100644 --- a/Grbl_Esp32/src/Configuration/AfterParse.cpp +++ b/Grbl_Esp32/src/Configuration/AfterParse.cpp @@ -19,12 +19,27 @@ #include "AfterParse.h" #include "Configurable.h" +#include "../System.h" +#include "../Logging.h" #include namespace Configuration { void AfterParse::enterSection(const char* name, Configurable* value) { - value->afterParse(); + _path.push_back(name); // For error handling + + try { + value->afterParse(); + } catch (const AssertionFailed& ex) { + // Log something meaningful to the user: + log_error("Initialization error at "; for (auto it : _path) { ss << '/' << it; } ss << ": " << ex.msg); + + // Set the state to config alarm, so users can't run time machine. + sys.state = State::ConfigAlarm; + } + value->group(*this); + + _path.erase(_path.begin() + (_path.size() - 1)); } } diff --git a/Grbl_Esp32/src/Configuration/AfterParse.h b/Grbl_Esp32/src/Configuration/AfterParse.h index 60bceb02..41a14d5a 100644 --- a/Grbl_Esp32/src/Configuration/AfterParse.h +++ b/Grbl_Esp32/src/Configuration/AfterParse.h @@ -18,11 +18,11 @@ #pragma once -#include - #include "../Pin.h" #include "HandlerBase.h" +#include + namespace Configuration { class Configurable; @@ -30,6 +30,8 @@ namespace Configuration { AfterParse(const AfterParse&) = delete; AfterParse& operator=(const AfterParse&) = delete; + std::vector _path; + protected: void enterSection(const char* name, Configurable* value) override; bool matchesUninitialized(const char* name) override { return false; } diff --git a/Grbl_Esp32/src/Configuration/ParserHandler.h b/Grbl_Esp32/src/Configuration/ParserHandler.h index 047f1734..c2493b32 100644 --- a/Grbl_Esp32/src/Configuration/ParserHandler.h +++ b/Grbl_Esp32/src/Configuration/ParserHandler.h @@ -24,15 +24,20 @@ #include "../Logging.h" +#include + // #define DEBUG_VERBOSE_YAML_PARSER // #define DEBUG_CHATTY_YAML_PARSER namespace Configuration { class ParserHandler : public Configuration::HandlerBase { private: - Configuration::Parser& _parser; + Configuration::Parser& _parser; + std::vector _path; public: void enterSection(const char* name, Configuration::Configurable* section) override { + _path.push_back(name); // For error handling + // On entry, the token is for the section that invoked us. // We will handle following nodes with indents greater than entryIndent int entryIndent = _parser.token_.indent_; @@ -66,7 +71,16 @@ namespace Configuration { #ifdef DEBUG_VERBOSE_YAML_PARSER log_debug("Parsing key " << _parser.key().str()); #endif - section->group(*this); + try { + section->group(*this); + } catch (const AssertionFailed& ex) { + // Log something meaningful to the user: + log_error("Configuration error at "; for (auto it : _path) { ss << '/' << it; } ss << ": " << ex.msg); + + // Set the state to config alarm, so users can't run time machine. + sys.state = State::ConfigAlarm; + } + if (_parser.token_.state == TokenState::Matching) { log_error("Ignored key " << _parser.key().str()); } @@ -90,6 +104,8 @@ namespace Configuration { #ifdef DEBUG_CHATTY_YAML_PARSER log_debug("Left section at indent " << entryIndent << " holding " << _parser.key().str()); #endif + + _path.erase(_path.begin() + (_path.size() - 1)); } bool matchesUninitialized(const char* name) override { return _parser.is(name); } diff --git a/Grbl_Esp32/src/Configuration/Validator.cpp b/Grbl_Esp32/src/Configuration/Validator.cpp index d78ecb92..a2a776c2 100644 --- a/Grbl_Esp32/src/Configuration/Validator.cpp +++ b/Grbl_Esp32/src/Configuration/Validator.cpp @@ -19,6 +19,8 @@ #include "Validator.h" #include "Configurable.h" +#include "../System.h" +#include "../Logging.h" #include #include @@ -30,7 +32,20 @@ namespace Configuration { } void Validator::enterSection(const char* name, Configurable* value) { - value->validate(); + _path.push_back(name); // For error handling + + try { + value->validate(); + } catch (const AssertionFailed& ex) { + // Log something meaningful to the user: + log_error("Validation error at "; for (auto it : _path) { ss << '/' << it; } ss << ": " << ex.msg); + + // Set the state to config alarm, so users can't run time machine. + sys.state = State::ConfigAlarm; + } + value->group(*this); + + _path.erase(_path.begin() + (_path.size() - 1)); } } diff --git a/Grbl_Esp32/src/Configuration/Validator.h b/Grbl_Esp32/src/Configuration/Validator.h index 6a170b00..a95ba137 100644 --- a/Grbl_Esp32/src/Configuration/Validator.h +++ b/Grbl_Esp32/src/Configuration/Validator.h @@ -18,11 +18,11 @@ #pragma once -#include - #include "../Pin.h" #include "HandlerBase.h" +#include + namespace Configuration { class Configurable; @@ -30,6 +30,8 @@ namespace Configuration { Validator(const Validator&) = delete; Validator& operator=(const Validator&) = delete; + std::vector _path; + protected: void enterSection(const char* name, Configurable* value) override; bool matchesUninitialized(const char* name) override { return false; } diff --git a/Grbl_Esp32/src/Error.cpp b/Grbl_Esp32/src/Error.cpp index b0401848..7720947d 100644 --- a/Grbl_Esp32/src/Error.cpp +++ b/Grbl_Esp32/src/Error.cpp @@ -82,4 +82,5 @@ std::map ErrorNames = { { Error::AnotherInterfaceBusy, "Another interface is busy" }, { Error::BadPinSpecification, "Bad Pin Specification" }, { Error::JogCancelled, "Jog Cancelled" }, + { Error::ConfigurationInvalid, "Configuration is invalid. Check boot messages for ERR's." }, }; diff --git a/Grbl_Esp32/src/Error.h b/Grbl_Esp32/src/Error.h index e4676f02..9f4e52d1 100644 --- a/Grbl_Esp32/src/Error.h +++ b/Grbl_Esp32/src/Error.h @@ -89,6 +89,7 @@ enum class Error : uint8_t { JogCancelled = 130, BadPinSpecification = 150, BadRuntimeConfigSetting = 151, + ConfigurationInvalid = 152, }; extern std::map ErrorNames; diff --git a/Grbl_Esp32/src/Grbl.cpp b/Grbl_Esp32/src/Grbl.cpp index 7869e012..d127e79b 100644 --- a/Grbl_Esp32/src/Grbl.cpp +++ b/Grbl_Esp32/src/Grbl.cpp @@ -113,26 +113,27 @@ void grbl_init() { } else { sys.state = State::Idle; } - } - // Check for power-up and set system alarm if homing is enabled to force homing cycle - // by setting Grbl's alarm state. Alarm locks out all g-code commands, including the - // startup scripts, but allows access to settings and internal commands. Only a homing - // cycle '$H' or kill alarm locks '$X' will disable the alarm. - // NOTE: The startup script will run after successful completion of the homing cycle, but - // not after disabling the alarm locks. Prevents motion startup blocks from crashing into - // things uncontrollably. Very bad. - if (config->_homingInitLock && homingAxes() && sys.state != State::ConfigAlarm) { - // If there is an axis with homing configured, enter Alarm state on startup - sys.state = State::Alarm; - } - for (auto s : config->_spindles) { - s->init(); - } - Spindles::Spindle::switchSpindle(0, config->_spindles, spindle); - config->_coolant->init(); - limits_init(); - config->_probe->init(); + // Check for power-up and set system alarm if homing is enabled to force homing cycle + // by setting Grbl's alarm state. Alarm locks out all g-code commands, including the + // startup scripts, but allows access to settings and internal commands. Only a homing + // cycle '$H' or kill alarm locks '$X' will disable the alarm. + // NOTE: The startup script will run after successful completion of the homing cycle, but + // not after disabling the alarm locks. Prevents motion startup blocks from crashing into + // things uncontrollably. Very bad. + if (config->_homingInitLock && homingAxes()) { + // If there is an axis with homing configured, enter Alarm state on startup + sys.state = State::Alarm; + } + for (auto s : config->_spindles) { + s->init(); + } + Spindles::Spindle::switchSpindle(0, config->_spindles, spindle); + + config->_coolant->init(); + limits_init(); + config->_probe->init(); + } WebUI::wifi_config.begin(); if (config->_comms->_bluetoothConfig) { @@ -155,11 +156,16 @@ static void reset_variables() { // or by the post-configuration fixup, but we test // it anyway just for safety. We want to avoid any // possibility of crashing at this point. - if (spindle) { - spindle->stop(); - } + plan_reset(); // Clear block buffer and planner variables - st_reset(); // Clear stepper subsystem variables + + if (sys.state != State::ConfigAlarm) { + if (spindle) { + spindle->stop(); + } + st_reset(); // Clear stepper subsystem variables + } + // Sync cleared gcode and planner positions to current system position. plan_sync_position(); gc_sync_position(); @@ -189,6 +195,7 @@ void run_once() { // require careful teardown of the existing configuration // to avoid memory leaks. It is probably worth doing eventually. error_all("Critical error in run_once: %s", ex.msg.c_str()); + error_all("Stacktrace: %s", ex.stackTrace.c_str()); sys.state = State::ConfigAlarm; } if (++tries > 1) { diff --git a/Grbl_Esp32/src/Machine/MachineConfig.cpp b/Grbl_Esp32/src/Machine/MachineConfig.cpp index 989d6c56..a744017f 100644 --- a/Grbl_Esp32/src/Machine/MachineConfig.cpp +++ b/Grbl_Esp32/src/Machine/MachineConfig.cpp @@ -276,7 +276,7 @@ namespace Machine { // log_info("Heap size after configuation load is " << uint32_t(xPortGetFreeHeapSize())); - successful = true; + successful = (sys.state != State::ConfigAlarm); } catch (const Configuration::ParseException& ex) { sys.state = State::ConfigAlarm; diff --git a/Grbl_Esp32/src/Pin.cpp b/Grbl_Esp32/src/Pin.cpp index adaf2e17..893367e5 100644 --- a/Grbl_Esp32/src/Pin.cpp +++ b/Grbl_Esp32/src/Pin.cpp @@ -158,10 +158,17 @@ Pin Pin::create(const StringRange& str) { return Pin(pinImplementation); } } catch (const AssertionFailed& ex) { // We shouldn't get here under normal circumstances. + + char buf[255]; + snprintf(buf, 255, "ERR: Setting up pin [%s] failed. Details: %s", str.str().c_str(), ex.what()); + + Assert(false, buf); + + /* pin_error("ERR: Setting up pin [%s] failed. Details: %s", str.str().c_str(), ex.what()); - (void)ex; // Get rid of compiler warning return Pin(new Pins::ErrorPinDetail(str.str())); + */ } } diff --git a/Grbl_Esp32/src/Pins/ErrorPinDetail.cpp b/Grbl_Esp32/src/Pins/ErrorPinDetail.cpp index a17bad68..d7ed09ee 100644 --- a/Grbl_Esp32/src/Pins/ErrorPinDetail.cpp +++ b/Grbl_Esp32/src/Pins/ErrorPinDetail.cpp @@ -51,5 +51,5 @@ namespace Pins { PinAttributes ErrorPinDetail::getAttr() const { return PinAttributes::None; } - String ErrorPinDetail::toString() { return "ERROR_PIN"; } + String ErrorPinDetail::toString() { return "ERROR_PIN (for " + _description + ")"; } } diff --git a/Grbl_Esp32/src/ProcessSettings.cpp b/Grbl_Esp32/src/ProcessSettings.cpp index 5a82bb7e..f22a390a 100644 --- a/Grbl_Esp32/src/ProcessSettings.cpp +++ b/Grbl_Esp32/src/ProcessSettings.cpp @@ -103,6 +103,10 @@ void settings_init() { // than actual settings, which change infrequently, so handling // it early is probably prudent. Error jog_set(uint8_t* value, WebUI::AuthenticationLevel auth_level, WebUI::ESPResponseStream* out) { + if (sys.state == State::ConfigAlarm) { + return Error::ConfigurationInvalid; + } + // Execute only if in IDLE or JOG states. if (sys.state != State::Idle && sys.state != State::Jog) { return Error::IdleError; @@ -217,6 +221,10 @@ Error list_commands(const char* value, WebUI::AuthenticationLevel auth_level, We return Error::Ok; } Error toggle_check_mode(const char* value, WebUI::AuthenticationLevel auth_level, WebUI::ESPResponseStream* out) { + if (sys.state == State::ConfigAlarm) { + return Error::ConfigurationInvalid; + } + // Perform reset when toggling off. Check g-code mode should only work if Grbl // is idle and ready, regardless of alarm locks. This is mainly to keep things // simple and consistent. @@ -234,7 +242,9 @@ Error toggle_check_mode(const char* value, WebUI::AuthenticationLevel auth_level return Error::Ok; } Error disable_alarm_lock(const char* value, WebUI::AuthenticationLevel auth_level, WebUI::ESPResponseStream* out) { - if (sys.state == State::Alarm || sys.state == State::ConfigAlarm) { + if (sys.state == State::ConfigAlarm) { + return Error::ConfigurationInvalid; + } else if (sys.state == State::Alarm) { // Block if safety door is ajar. if (config->_control->system_check_safety_door_ajar()) { return Error::CheckDoor; @@ -250,6 +260,10 @@ Error report_ngc(const char* value, WebUI::AuthenticationLevel auth_level, WebUI return Error::Ok; } Error home(int cycle) { + if (sys.state == State::ConfigAlarm) { + return Error::ConfigurationInvalid; + } + if (!homingAxes()) { return Error::SettingDisabled; } @@ -346,6 +360,10 @@ Error showState(const char* value, WebUI::AuthenticationLevel auth_level, WebUI: } Error doJog(const char* value, WebUI::AuthenticationLevel auth_level, WebUI::ESPResponseStream* out) { + if (sys.state == State::ConfigAlarm) { + return Error::ConfigurationInvalid; + } + // For jogging, you must give gc_execute_line() a line that // begins with $J=. There are several ways we can get here, // including $J, $J=xxx, [J]xxx. For any form other than @@ -421,6 +439,10 @@ Error listErrors(const char* value, WebUI::AuthenticationLevel auth_level, WebUI } Error motor_disable(const char* value, WebUI::AuthenticationLevel auth_level, WebUI::ESPResponseStream* out) { + if (sys.state == State::ConfigAlarm) { + return Error::ConfigurationInvalid; + } + while (value && isspace(*value)) { ++value; } @@ -556,7 +578,7 @@ Error do_command_or_setting(const char* key, char* value, WebUI::AuthenticationL config->group(validator); } catch (std::exception& ex) { log_error("Validation error: " << ex.what()); - return Error::BadRuntimeConfigSetting; + return Error::ConfigurationInvalid; } Configuration::AfterParse afterParseHandler; @@ -567,10 +589,10 @@ Error do_command_or_setting(const char* key, char* value, WebUI::AuthenticationL } } catch (const Configuration::ParseException& ex) { log_error("Configuration parse error: " << ex.What() << " @ " << ex.LineNumber() << ":" << ex.ColumnNumber()); - return Error::BadRuntimeConfigSetting; + return Error::ConfigurationInvalid; } catch (const AssertionFailed& ex) { log_error("Configuration change failed: " << ex.what()); - return Error::BadRuntimeConfigSetting; + return Error::ConfigurationInvalid; } // Next search the settings list by text name. If found, set a new diff --git a/Grbl_Esp32/src/Protocol.cpp b/Grbl_Esp32/src/Protocol.cpp index 4f60e863..e6c71cff 100644 --- a/Grbl_Esp32/src/Protocol.cpp +++ b/Grbl_Esp32/src/Protocol.cpp @@ -132,32 +132,35 @@ void protocol_main_loop() { empty_lines(); //uint8_t client = CLIENT_SERIAL; // default client - // Perform some machine checks to make sure everything is good to go. - if (config->_checkLimitsAtInit && config->_axes->hasHardLimits()) { - if (limits_get_state()) { - sys.state = State::Alarm; // Ensure alarm state is active. - report_feedback_message(Message::CheckLimits); - } - } - // Check for and report alarm state after a reset, error, or an initial power up. // NOTE: Sleep mode disables the stepper drivers and position can't be guaranteed. // Re-initialize the sleep state as an ALARM mode to ensure user homes or acknowledges. if (sys.state == State::ConfigAlarm) { report_feedback_message(Message::ConfigAlarmLock); - } else if (sys.state == State::Alarm || sys.state == State::Sleep) { - report_feedback_message(Message::AlarmLock); - sys.state = State::Alarm; // Ensure alarm state is set. } else { - // Check if the safety door is open. - sys.state = State::Idle; - if (config->_control->system_check_safety_door_ajar()) { - rtSafetyDoor = true; - protocol_execute_realtime(); // Enter safety door mode. Should return as IDLE state. + // Perform some machine checks to make sure everything is good to go. + if (config->_checkLimitsAtInit && config->_axes->hasHardLimits()) { + if (limits_get_state()) { + sys.state = State::Alarm; // Ensure alarm state is active. + report_feedback_message(Message::CheckLimits); + } + } + + if (sys.state == State::Alarm || sys.state == State::Sleep) { + report_feedback_message(Message::AlarmLock); + sys.state = State::Alarm; // Ensure alarm state is set. + } else { + // Check if the safety door is open. + sys.state = State::Idle; + if (config->_control->system_check_safety_door_ajar()) { + rtSafetyDoor = true; + protocol_execute_realtime(); // Enter safety door mode. Should return as IDLE state. + } + // All systems go! + system_execute_startup(); // Execute startup script. } - // All systems go! - system_execute_startup(); // Execute startup script. } + // --------------------------------------------------------------------------------- // Primary loop! Upon a system abort, this exits back to main() to reset the system. // This is also where Grbl idles while waiting for something to do. diff --git a/Grbl_Esp32/src/Report.cpp b/Grbl_Esp32/src/Report.cpp index ee570f49..9abd4cb3 100644 --- a/Grbl_Esp32/src/Report.cpp +++ b/Grbl_Esp32/src/Report.cpp @@ -295,7 +295,7 @@ std::map MessageText = { { Message::RestoreDefaults, "Restoring defaults" }, { Message::SpindleRestore, "Restoring spindle" }, { Message::SleepMode, "Sleeping" }, - { Message::ConfigAlarmLock, "Configuration error - '$X' to unlock" }, + { Message::ConfigAlarmLock, "Configuration is invalid. Check boot messages for ERR's." }, // Handled separately due to numeric argument // { Message::SdFileQuit, "Reset during SD file at line: %d" }, }; diff --git a/Grbl_Esp32/src/Spindles/Spindle.cpp b/Grbl_Esp32/src/Spindles/Spindle.cpp index 32f5b185..09b7f022 100644 --- a/Grbl_Esp32/src/Spindles/Spindle.cpp +++ b/Grbl_Esp32/src/Spindles/Spindle.cpp @@ -137,7 +137,7 @@ namespace Spindles { dev_speed += (((speed - _speeds[i].speed) * _speeds[i].scale) >> 16); } - log_debug("rpm " << speed << " speed " << dev_speed); + // log_debug("rpm " << speed << " speed " << dev_speed); // This will spew quite a bit of data on your output return dev_speed; } void Spindle::spinDelay(SpindleState state, SpindleSpeed speed) { diff --git a/Grbl_Esp32/src/Stepper.cpp b/Grbl_Esp32/src/Stepper.cpp index 8de4da53..0ffe0d81 100644 --- a/Grbl_Esp32/src/Stepper.cpp +++ b/Grbl_Esp32/src/Stepper.cpp @@ -198,7 +198,7 @@ static void Stepper_Timer_Stop(); // Stepper timer configuration const int stepTimerNumber = 0; -hw_timer_t* stepTimer; // Handle +hw_timer_t* stepTimer = nullptr; // Handle // autoReload true might give better step timing - but it also // might cause problems if an interrupt takes too long const bool autoReload = true; @@ -958,7 +958,7 @@ static void IRAM_ATTR Stepper_Timer_Start() { static void IRAM_ATTR Stepper_Timer_Stop() { if (config->_stepType == ST_I2S_STREAM) { i2s_out_set_passthrough(); - } else { + } else if (stepTimer) { timerAlarmDisable(stepTimer); } }