Re-organized status messages to be more coherent and centralized.

- Reorganized all of the status message feedback from both the g-code
parser and settings modules to be centralized into two message modules:
status feedback from executing a line and warnings for misc feedback.

- Pulled out the printPgmString() messages in settings.c and placed it
into the new module. (settings_dump() not moved).

- Some other minor edits. Renaming defines, comment updates, etc.

config.h

@@ -150,9 +150,10 @@
 // of your successes or difficulties, as we will monitor this and possibly integrate this as a 
 // standard feature for future releases. However, we suggest to first try our direction delay
 // hack/solution posted in the Wiki involving inverting the stepper pin mask.
-// NOTE: Uncomment to enable. The recommended delay should be > 3us and the total step pulse
+// NOTE: Uncomment to enable. The recommended delay must be > 3us and the total step pulse
-// time, which includes the Grbl settings pulse microseconds, should not exceed 127us.
+// time, which includes the Grbl settings pulse microseconds, must not exceed 127us. Reported
-// #define STEP_PULSE_DELAY 5 // Step pulse delay in microseconds. Default disabled.
+// successful values for certain setups have ranged from 10 to 20us.
+// #define STEP_PULSE_DELAY 10 // Step pulse delay in microseconds. Default disabled.
 
 // ---------------------------------------------------------------------------------------
 
@@ -167,7 +168,8 @@
 // entering g-code into grbl, i.e. locating part zero or simple manual machining. If the axes drift,
 // grbl has no way to know this has happened, since stepper motors are open-loop control. Depending
 // on the machine, this parameter may need to be larger or smaller than the default time.
-// NOTE: If the define commented, the stepper lock will be disabled upon compiling.
+// NOTE: If set to zero, no lock will occur. If set to max 255, the lock will never release, in other
+// words, the steppers never disable for users that require this.
 // -> NOW INSTALLED IN SETTINGS #define STEPPER_IDLE_LOCK_TIME 25 // (milliseconds) - Integer > 0
 
 

gcode.c

@@ -265,7 +265,7 @@ uint8_t gc_execute_line(char *line)
     switch(letter) {
       case 'G': case 'M': case 'N': break; // Ignore command statements and line numbers
       case 'F': 
-        if (value <= 0) { FAIL(STATUS_INVALID_COMMAND); } // Must be greater than zero
+        if (value <= 0) { FAIL(STATUS_INVALID_STATEMENT); } // Must be greater than zero
         if (gc.inverse_feed_rate_mode) {
           inverse_feed_rate = to_millimeters(value); // seconds per motion for this motion only
         } else {          
@@ -277,11 +277,11 @@ uint8_t gc_execute_line(char *line)
       case 'P': p = value; break;                    
       case 'R': r = to_millimeters(value); break;
       case 'S': 
-        if (value < 0) { FAIL(STATUS_INVALID_COMMAND); } // Cannot be negative
+        if (value < 0) { FAIL(STATUS_INVALID_STATEMENT); } // Cannot be negative
         gc.spindle_speed = value;
         break;
       case 'T': 
-        if (value < 0) { FAIL(STATUS_INVALID_COMMAND); } // Cannot be negative
+        if (value < 0) { FAIL(STATUS_INVALID_STATEMENT); } // Cannot be negative
         gc.tool = trunc(value); 
         break;
       case 'X': target[X_AXIS] = to_millimeters(value); bit_true(axis_words,bit(X_AXIS)); break;
@@ -299,7 +299,7 @@ uint8_t gc_execute_line(char *line)
      NOTE: Independent non-motion/settings parameters are set out of this order for code efficiency 
      and simplicity purposes, but this should not affect proper g-code execution. */
   
-  //  ([M6]: Tool change execution should be executed here.)
+  //  ([M6]: Tool change should be executed here.)
   
   // [M3,M4,M5]: Update spindle state
   spindle_run(gc.spindle_direction, gc.spindle_speed);
@@ -313,7 +313,7 @@ uint8_t gc_execute_line(char *line)
   switch (non_modal_action) {
     case NON_MODAL_DWELL:
       if (p < 0) { // Time cannot be negative.
-        FAIL(STATUS_INVALID_COMMAND); 
+        FAIL(STATUS_INVALID_STATEMENT); 
       } else { 
         mc_dwell(p); 
       }
@@ -323,7 +323,7 @@ uint8_t gc_execute_line(char *line)
       if (l != 2 || (int_value < 1 || int_value > N_COORDINATE_SYSTEM)) { // L2 only. P1=G54, P2=G55, ... 
         FAIL(STATUS_UNSUPPORTED_STATEMENT); 
       } else if (!axis_words) { // No axis words.
-        FAIL(STATUS_INVALID_COMMAND);
+        FAIL(STATUS_INVALID_STATEMENT);
       } else {
         int_value--; // Adjust p to be inline with row array index. 
         // Update axes defined only in block. Always in machine coordinates. Can change non-active system.
@@ -358,7 +358,7 @@ uint8_t gc_execute_line(char *line)
       break;      
     case NON_MODAL_SET_COORDINATE_OFFSET:
       if (!axis_words) { // No axis words
-        FAIL(STATUS_INVALID_COMMAND);
+        FAIL(STATUS_INVALID_STATEMENT);
       } else {
         // Update axes defined only in block. Offsets current system to defined value. Does not update when
         // active coordinate system is selected, but is still active unless G92.1 disables it. 
@@ -384,12 +384,12 @@ uint8_t gc_execute_line(char *line)
     // G1,G2,G3 require F word in inverse time mode.  
     if ( gc.inverse_feed_rate_mode ) { 
       if (inverse_feed_rate < 0 && gc.motion_mode != MOTION_MODE_CANCEL) {
-        FAIL(STATUS_INVALID_COMMAND);
+        FAIL(STATUS_INVALID_STATEMENT);
       }
     }
     // Absolute override G53 only valid with G0 and G1 active.
     if ( absolute_override && !(gc.motion_mode == MOTION_MODE_SEEK || gc.motion_mode == MOTION_MODE_LINEAR)) {
-      FAIL(STATUS_INVALID_COMMAND);
+      FAIL(STATUS_INVALID_STATEMENT);
     }
     // Report any errors.  
     if (gc.status_code) { return(gc.status_code); }
@@ -414,14 +414,14 @@ uint8_t gc_execute_line(char *line)
   
     switch (gc.motion_mode) {
       case MOTION_MODE_CANCEL: 
-        if (axis_words) { FAIL(STATUS_INVALID_COMMAND); } // No axis words allowed while active.
+        if (axis_words) { FAIL(STATUS_INVALID_STATEMENT); } // No axis words allowed while active.
         break;
       case MOTION_MODE_SEEK:
-        if (!axis_words) { FAIL(STATUS_INVALID_COMMAND);} 
+        if (!axis_words) { FAIL(STATUS_INVALID_STATEMENT);} 
         else { mc_line(target[X_AXIS], target[Y_AXIS], target[Z_AXIS], settings.default_seek_rate, false); }
         break;
       case MOTION_MODE_LINEAR:
-        if (!axis_words) { FAIL(STATUS_INVALID_COMMAND);} 
+        if (!axis_words) { FAIL(STATUS_INVALID_STATEMENT);} 
         else { mc_line(target[X_AXIS], target[Y_AXIS], target[Z_AXIS], 
           (gc.inverse_feed_rate_mode) ? inverse_feed_rate : gc.feed_rate, gc.inverse_feed_rate_mode); }
         break;
@@ -430,7 +430,7 @@ uint8_t gc_execute_line(char *line)
         // format arc mode, also check for at least one of the IJK axes of the selected plane was sent.
         if ( !( bit_false(axis_words,bit(gc.plane_axis_2)) ) || 
              ( !r && !offset[gc.plane_axis_0] && !offset[gc.plane_axis_1] ) ) { 
-          FAIL(STATUS_INVALID_COMMAND);
+          FAIL(STATUS_INVALID_STATEMENT);
         } else {
           if (r != 0) { // Arc Radius Mode
             /* 

protocol.c

@@ -36,34 +36,70 @@ static char line[LINE_BUFFER_SIZE]; // Line to be executed. Zero-terminated.
 static uint8_t char_counter; // Last character counter in line variable.
 static uint8_t iscomment; // Comment/block delete flag for processor to ignore comment characters.
 
+// Prints all status messages, an 'ok' or 'error', after Grbl has processed a line of incoming
+// serial data, whether this was a g-code block or grbl setting command.
 void protocol_status_message(int8_t status_code) 
 {
-  if (status_code == 0) {
+  // TODO: Compile time option to only return numeric codes for GUIs.
+  if (status_code == 0) { // STATUS_OK
     printPgmString(PSTR("ok\r\n"));
   } else {
     printPgmString(PSTR("error: "));
+    // All critical error codes are greater than zero. These are defined to be any error
+    // that may cause damage by crashing or improper g-code inputs and that are susceptible
+    // to Grbl's alarm mode which will stop all processes, if the user enables this option.
+    if (status_code > 0) { 
+      // TODO: Install option to enter alarm mode upon any critical error.
       switch(status_code) {          
         case STATUS_BAD_NUMBER_FORMAT:
-      printPgmString(PSTR("Bad number format\r\n")); break;
+        printPgmString(PSTR("Bad number format")); break;
         case STATUS_EXPECTED_COMMAND_LETTER:
-      printPgmString(PSTR("Expected command letter\r\n")); break;
+        printPgmString(PSTR("Expected command letter")); break;
         case STATUS_UNSUPPORTED_STATEMENT:
-      printPgmString(PSTR("Unsupported statement\r\n")); break;
+        printPgmString(PSTR("Unsupported statement")); break;
         case STATUS_FLOATING_POINT_ERROR:
-      printPgmString(PSTR("Floating point error\r\n")); break;
+        printPgmString(PSTR("Floating point error")); break;
         case STATUS_MODAL_GROUP_VIOLATION:
-      printPgmString(PSTR("Modal group violation\r\n")); break;
+        printPgmString(PSTR("Modal group violation")); break;
-      case STATUS_INVALID_COMMAND:
+        case STATUS_INVALID_STATEMENT:
-      printPgmString(PSTR("Invalid command\r\n")); break;
+        printPgmString(PSTR("Invalid gcode statement")); break;
         case STATUS_SETTING_DISABLED:
-      printPgmString(PSTR("Grbl setting disabled\r\n")); break;
+        printPgmString(PSTR("Grbl setting disabled")); break;
         case STATUS_HARD_LIMIT:
-      printPgmString(PSTR("Limit triggered <Check and Reset>\r\n")); break;
+        printPgmString(PSTR("Limit triggered <Check and Reset>")); break;
-      default:
+      }
-      printInteger(status_code);
+    // All other non-critical error codes are less than zero. These are defined to be any
+    // error that is not susceptible to the alarm mode. Typically settings responses.
+    } else {
+      switch(status_code) {
+        case STATUS_SETTING_INVALID:
+        printPgmString(PSTR("Invalid setting statement")); break;
+        case STATUS_SETTING_STEPS_NEG:
+        printPgmString(PSTR("Steps/mm must be > 0.0")); break;
+        case STATUS_SETTING_STEP_PULSE_MIN:
+        printPgmString(PSTR("Step pulse must be >= 3 microseconds")); break;
+      }
+    }
     printPgmString(PSTR("\r\n"));
   }
+}
+
+
+// Prints Grbl warning messages. This serves as a centralized method to provide additional
+// user feedback for things that do not pass through the protocol_execute_line() function.
+// This includes things like initialization checks or setup warnings when features are
+// enabled. This function maybe called from anywhere in Grbl at the point of concern.
+void protocol_warning_message(int8_t warning_code)
+{
+  // TODO: Install silence warning messages option in settings
+  printPgmString(PSTR("warning: "));
+  switch(warning_code) {
+    case WARNING_HOMING_ENABLE:
+      printPgmString(PSTR("Install all axes limit switches before use")); break;
+    case WARNING_SETTING_READ_FAIL:
+      printPgmString(PSTR("Failed to read EEPROM settings. Using defaults")); break;
   }
+  printPgmString(PSTR("\r\n"));
 }
 
 
@@ -99,10 +135,11 @@ void protocol_status_report()
    if (bit_istrue(settings.flags,BITFLAG_REPORT_INCHES)) { print_position[i] *= INCH_PER_MM; }
    printFloat(print_position[i]);
    if (i < 2) { printPgmString(PSTR(",")); }
+   else { printPgmString(PSTR("]")); }
  }
  
  // Report work position
- printPgmString(PSTR("],WPos:[")); 
+ printPgmString(PSTR(",WPos:[")); 
  for (i=0; i<= 2; i++) {
    if (bit_istrue(settings.flags,BITFLAG_REPORT_INCHES)) {
      print_position[i] -= (sys.coord_system[sys.coord_select][i]+sys.coord_offset[i])*INCH_PER_MM;
@@ -111,9 +148,10 @@ void protocol_status_report()
    }
    printFloat(print_position[i]);
    if (i < 2) { printPgmString(PSTR(",")); }
+   else { printPgmString(PSTR("]")); }
  }
    
- printPgmString(PSTR("]\r\n"));
+ printPgmString(PSTR("\r\n"));
 }
 
 
@@ -133,7 +171,7 @@ void protocol_init()
 // point where the execution time from the last check point may be more than a fraction of a second.
 // This is a way to execute runtime commands asynchronously (aka multitasking) with grbl's g-code
 // parsing and planning functions. This function also serves as an interface for the interrupts to 
-// set the system runtime flags, where only the main program to handles them, removing the need to
+// set the system runtime flags, where only the main program handles them, removing the need to
 // define more computationally-expensive volatile variables.
 // NOTE: The sys.execute variable flags are set by the serial read subprogram, except where noted.
 void protocol_execute_runtime()
@@ -185,7 +223,7 @@ void protocol_execute_runtime()
 
 
 // Executes one line of input according to protocol
-uint8_t protocol_execute_line(char *line) 
+int8_t protocol_execute_line(char *line) 
 {     
   if(line[0] == '$') {
   

protocol.h

@@ -21,17 +21,28 @@
 #ifndef protocol_h
 #define protocol_h
 
+
+#define LINE_BUFFER_SIZE 50
+
+// Define Grbl status codes.
 #define STATUS_OK 0
+// Critical error codes. Greater than zero.
 #define STATUS_BAD_NUMBER_FORMAT 1
 #define STATUS_EXPECTED_COMMAND_LETTER 2
 #define STATUS_UNSUPPORTED_STATEMENT 3
 #define STATUS_FLOATING_POINT_ERROR 4
 #define STATUS_MODAL_GROUP_VIOLATION 5
-#define STATUS_INVALID_COMMAND 6
+#define STATUS_INVALID_STATEMENT 6
 #define STATUS_SETTING_DISABLED 7
 #define STATUS_HARD_LIMIT 8
+// Non-critical error codes. Less than zero.
+#define STATUS_SETTING_INVALID -1
+#define STATUS_SETTING_STEPS_NEG -2
+#define STATUS_SETTING_STEP_PULSE_MIN -3
 
-#define LINE_BUFFER_SIZE 50
+// Define Grbl warning message codes
+#define WARNING_HOMING_ENABLE 1
+#define WARNING_SETTING_READ_FAIL 2
 
 // Initialize the serial protocol
 void protocol_init();
@@ -41,12 +52,12 @@ void protocol_init();
 void protocol_process();
 
 // Executes one line of input according to protocol
-uint8_t protocol_execute_line(char *line);
+int8_t protocol_execute_line(char *line);
 
 // Checks and executes a runtime command at various stop points in main program
 void protocol_execute_runtime();
 
-// Prints g-code parser status message.
+// Prints any warning messages.
-void protocol_status_message(int8_t status_code);
+void protocol_warning_message(int8_t warning_code);
 
 #endif

serial.c

@@ -169,6 +169,7 @@ ISR(USART_RX_vect)
       sys.execute |= EXEC_ALARM; // Set alarm to allow subsystem disable for certain settings.
       
       // TODO: When Grbl system status is installed, set position lost state if the cycle is active.
+      // if (sys.cycle_start) { POSITION LOST } 
       
       // Immediately force stepper and spindle subsystem idle at an interrupt level.
       if (!(sys.execute & EXEC_RESET)) { // Force stop only first time.

settings.c

@@ -147,13 +147,11 @@ void settings_dump() {
 }
 
 // Parameter lines are on the form '$4=374.3' or '$' to dump current settings
-uint8_t settings_execute_line(char *line) {
+// NOTE: Assumes '$' already exists in line[0], which is checked by protocol.c.
+int8_t settings_execute_line(char *line) {
   uint8_t char_counter = 1; 
 //  unsigned char letter;
   float parameter, value;
-  if(line[0] != '$') { 
-    return(STATUS_UNSUPPORTED_STATEMENT); 
-  }
   if(line[char_counter] == 0) { 
     settings_dump(); return(STATUS_OK); 
   }
@@ -175,19 +173,18 @@ uint8_t settings_execute_line(char *line) {
 //     
 //   } else {
   if(!read_float(line, &char_counter, &parameter)) {
-    return(STATUS_BAD_NUMBER_FORMAT);
+    return(STATUS_SETTING_INVALID);
-  };
+  }
   if(line[char_counter++] != '=') { 
-    return(STATUS_UNSUPPORTED_STATEMENT); 
+    return(STATUS_SETTING_INVALID); 
   }
   if(!read_float(line, &char_counter, &value)) {
-    return(STATUS_BAD_NUMBER_FORMAT);
+    return(STATUS_SETTING_INVALID);
   }
   if(line[char_counter] != 0) { 
-    return(STATUS_UNSUPPORTED_STATEMENT); 
+    return(STATUS_SETTING_INVALID); 
   }
-  settings_store_setting(parameter, value);
+  return(settings_store_setting(parameter, value));
-  return(STATUS_OK);
 //   }
 }
 
@@ -250,19 +247,13 @@ int read_settings() {
 }
 
 // A helper method to set settings from command line
-void settings_store_setting(int parameter, float value) {
+int8_t settings_store_setting(int parameter, float value) {
   switch(parameter) {
     case 0: case 1: case 2:
-    if (value <= 0.0) {
+      if (value <= 0.0) { return(STATUS_SETTING_STEPS_NEG); } 
-      printPgmString(PSTR("Steps/mm must be > 0.0\r\n"));
-      return;
-    }
       settings.steps_per_mm[parameter] = value; break;
     case 3: 
-    if (value < 3) {
+      if (value < 3) { return(STATUS_SETTING_STEP_PULSE_MIN); }
-      printPgmString(PSTR("Step pulse must be >= 3 microseconds\r\n"));
-      return;
-    }
       settings.pulse_microseconds = round(value); break;
     case 4: settings.default_feed_rate = value; break;
     case 5: settings.default_seek_rate = value; break;
@@ -288,7 +279,7 @@ void settings_store_setting(int parameter, float value) {
     case 13:
       if (value) { 
         settings.flags |= BITFLAG_HOMING_ENABLE; 
-        printPgmString(PSTR("Install all axes limit switches before use\r\n")); 
+        protocol_warning_message(WARNING_HOMING_ENABLE);
       } else { settings.flags &= ~BITFLAG_HOMING_ENABLE; }
       break;
     case 14: settings.homing_dir_mask = trunc(value); break;
@@ -302,17 +293,16 @@ void settings_store_setting(int parameter, float value) {
        break;
     case 20: settings.decimal_places = round(value); break;
     default: 
-      printPgmString(PSTR("Unknown parameter\r\n"));
+      return(STATUS_SETTING_INVALID);
-      return;
   }
   write_settings();
-  printPgmString(PSTR("Stored new setting\r\n"));
+  return(STATUS_OK);
 }
 
 // Initialize the config subsystem
 void settings_init() {
   if(!read_settings()) {
-    printPgmString(PSTR("Warning: Failed to read EEPROM settings. Using defaults.\r\n"));
+    protocol_warning_message(WARNING_SETTING_READ_FAIL);
     settings_reset(true);
     write_settings();
     settings_dump();

settings.h

@@ -31,7 +31,7 @@
 // when firmware is upgraded. Always stored in byte 0 of eeprom
 #define SETTINGS_VERSION 55
 
-// Define bit flag masks in settings.flag.
+// Define bit flag masks for the boolean settings in settings.flag.
 #define BITFLAG_REPORT_INCHES      bit(0)
 #define BITFLAG_AUTO_START         bit(1)
 #define BITFLAG_HARD_LIMIT_ENABLE  bit(2)
@@ -48,13 +48,13 @@ typedef struct {
   float mm_per_arc_segment;
   float acceleration;
   float junction_deviation;
-  uint8_t flags;  // Contains default toggles
+  uint8_t flags;  // Contains default boolean settings
   uint8_t homing_dir_mask;
   float homing_feed_rate;
   float homing_seek_rate;
   uint16_t homing_debounce_delay;
   float homing_pulloff;
-  uint8_t stepper_idle_lock_time;
+  uint8_t stepper_idle_lock_time; // If max value 255, steppers do not disable.
   uint8_t decimal_places; 
 } settings_t;
 extern settings_t settings;
@@ -66,10 +66,10 @@ void settings_init();
 void settings_dump();
 
 // Handle settings command
-uint8_t settings_execute_line(char *line);
+int8_t settings_execute_line(char *line);
 
 // A helper method to set new settings from command line
-void settings_store_setting(int parameter, float value);
+int8_t settings_store_setting(int parameter, float value);
 
 // int8_t settings_execute_startup();