Merge pull request #9327 from thinkyhead/bf2_fix_parser_M118

[2.0.x] Fix parser.has_value, M118 parameters

Marlin/src/HAL/HAL_LPC1768/persistent_store_impl.cpp

@@ -107,7 +107,7 @@ bool write_data(int &pos, const uint8_t *value, uint16_t size, uint16_t *crc) {
    SERIAL_PROTOCOLPAIR(" write_data(", pos);         // This extra chit-chat goes away soon.  But it is helpful
    SERIAL_PROTOCOLPAIR(",", (int)value);            // right now to see errors that are happening in the
    SERIAL_PROTOCOLPAIR(",", (int)size);             // read_data() and write_data() functions
-   SERIAL_PROTOCOL("...)\n");
+   SERIAL_PROTOCOLLNPGM("...)");
    SERIAL_PROTOCOLLNPAIR(" f_lseek()=", (int)s);
    return s;
   }
@@ -117,7 +117,7 @@ bool write_data(int &pos, const uint8_t *value, uint16_t size, uint16_t *crc) {
    SERIAL_PROTOCOLPAIR(" write_data(", pos);         // This extra chit-chat goes away soon.  But it is helpful
    SERIAL_PROTOCOLPAIR(",", (int)value);            // right now to see errors that are happening in the
    SERIAL_PROTOCOLPAIR(",", size);             // read_data() and write_data() functions
-   SERIAL_PROTOCOLLN("...)");
+   SERIAL_PROTOCOLLNPGM("...)");
    SERIAL_PROTOCOLLNPAIR(" f_write()=", (int)s);
    SERIAL_PROTOCOLPAIR(" size=", size);
    SERIAL_PROTOCOLLNPAIR("\n bytes_written=", bytes_written);
@@ -136,7 +136,7 @@ bool read_data(int &pos, uint8_t* value, uint16_t size, uint16_t *crc, const boo
    SERIAL_PROTOCOLPAIR(" read_data(", pos);          // This extra chit-chat goes away soon.  But it is helpful
    SERIAL_PROTOCOLPAIR(",", (int)value);            // right now to see errors that are happening in the
    SERIAL_PROTOCOLPAIR(",", size);             // read_data() and write_data() functions
-   SERIAL_PROTOCOLLN("...)");
+   SERIAL_PROTOCOLLNPGM("...)");
    SERIAL_PROTOCOLLNPAIR(" f_lseek()=", (int)s);
    return true;
   }
@@ -153,7 +153,7 @@ bool read_data(int &pos, uint8_t* value, uint16_t size, uint16_t *crc, const boo
    SERIAL_PROTOCOLPAIR(" read_data(", pos);         // This extra chit-chat goes away soon.  But it is helpful
    SERIAL_PROTOCOLPAIR(",", (int)value);           // right now to see errors that are happening in the
    SERIAL_PROTOCOLPAIR(",", size);            // read_data() and write_data() functions
-   SERIAL_PROTOCOLLN("...)");
+   SERIAL_PROTOCOLLNPGM("...)");
    SERIAL_PROTOCOLLNPAIR(" f_write()=", (int)s);
    SERIAL_PROTOCOLPAIR(" size=", size);
    SERIAL_PROTOCOLLNPAIR("\n bytes_read=",  bytes_read);

Marlin/src/HAL/HAL_STM32F7/HAL_STM32F7.h

@@ -98,7 +98,7 @@
   #define NUM_SERIAL 1
 #endif
 
-#define _BV(bit) (1 << (bit))
+#define _BV(b) (1UL << (b))
 
 /**
  * TODO: review this to return 1 for pins that are not analog input

Marlin/src/HAL/HAL_STM32F7/fastio_STM32F7.h

@@ -29,7 +29,7 @@
 #ifndef _FASTIO_STM32F7_H
 #define _FASTIO_STM32F7_H
 
-#define _BV(bit) (1 << (bit))
+#define _BV(b) (1UL << (b))
 
 #define READ(IO)              digitalRead(IO)
 #define WRITE(IO, v)          digitalWrite(IO,v)

Marlin/src/core/macros.h

@@ -95,9 +95,8 @@
 #define STRINGIFY(M) STRINGIFY_(M)
 
 // Macros for bit masks
-#ifndef _BV
+#undef _BV // Marlin needs 32-bit unsigned!
-  #define _BV(n)  (1<<(n))
+#define _BV(b) (1UL << (b))
-#endif
 #define TEST(n,b) (((n)&_BV(b))!=0)
 #define SBI(n,b) (n |= _BV(b))
 #define CBI(n,b) (n &= ~_BV(b))

Marlin/src/feature/bedlevel/ubl/ubl_G29.cpp

@@ -1095,7 +1095,7 @@
     SERIAL_PROTOCOLLNPAIR("UBL object count: ", (int)ubl_cnt);
 
     #if ENABLED(ENABLE_LEVELING_FADE_HEIGHT)
-      SERIAL_PROTOCOL("planner.z_fade_height : ");
+      SERIAL_PROTOCOLPGM("planner.z_fade_height : ");
       SERIAL_PROTOCOL_F(planner.z_fade_height, 4);
       SERIAL_EOL();
     #endif

Marlin/src/feature/dac/stepper_dac.cpp

@@ -114,7 +114,7 @@ void dac_print_values() {
   SERIAL_ECHOPAIR(" (",   dac_amps(Z_AXIS));
   SERIAL_ECHOPAIR(") E:", dac_perc(E_AXIS));
   SERIAL_ECHOPAIR(" (",   dac_amps(E_AXIS));
-  SERIAL_ECHOLN(")");
+  SERIAL_ECHOLNPGM(")");
 }
 
 void dac_commit_eeprom() {

Marlin/src/gcode/config/M301.cpp

@@ -71,7 +71,7 @@ void GcodeSuite::M301() {
   }
   else {
     SERIAL_ERROR_START();
-    SERIAL_ERRORLN(MSG_INVALID_EXTRUDER);
+    SERIAL_ERRORLNPGM(MSG_INVALID_EXTRUDER);
   }
 }
 

Marlin/src/gcode/feature/caselight/M355.cpp

@@ -56,10 +56,10 @@ void GcodeSuite::M355() {
     // always report case light status
     SERIAL_ECHO_START();
     if (!case_light_on) {
-      SERIAL_ECHOLN("Case light: off");
+      SERIAL_ECHOLNPGM("Case light: off");
     }
     else {
-      if (!USEABLE_HARDWARE_PWM(CASE_LIGHT_PIN)) SERIAL_ECHOLN("Case light: on");
+      if (!USEABLE_HARDWARE_PWM(CASE_LIGHT_PIN)) SERIAL_ECHOLNPGM("Case light: on");
       else SERIAL_ECHOLNPAIR("Case light: ", case_light_brightness);
     }
   #else

Marlin/src/gcode/feature/i2c/M260_M261.cpp

@@ -73,7 +73,7 @@ void GcodeSuite::M261() {
   }
   else {
     SERIAL_ERROR_START();
-    SERIAL_ERRORLN("Bad i2c request");
+    SERIAL_ERRORLNPGM("Bad i2c request");
   }
 }
 

Marlin/src/gcode/host/M118.cpp

@@ -29,7 +29,7 @@
  *  E1  Have the host 'echo:' the text
  */
 void GcodeSuite::M118() {
-  if (parser.boolval('E')) SERIAL_ECHO_START();
+  if (parser.seenval('E') && parser.value_bool()) SERIAL_ECHO_START();
-  if (parser.boolval('A')) SERIAL_ECHOPGM("// ");
+  if (parser.seenval('A') && parser.value_bool()) SERIAL_ECHOPGM("// ");
   SERIAL_ECHOLN(parser.string_arg);
 }

Marlin/src/gcode/parser.cpp

@@ -60,7 +60,7 @@ int GCodeParser::codenum;
 
 #if ENABLED(FASTER_GCODE_PARSER)
   // Optimized Parameters
-  byte GCodeParser::codebits[4];   // found bits
+  uint32_t GCodeParser::codebits;  // found bits
   uint8_t GCodeParser::param[26];  // parameter offsets from command_ptr
 #else
   char *GCodeParser::command_args; // start of parameters
@@ -83,7 +83,7 @@ void GCodeParser::reset() {
     subcode = 0;                        // No command sub-code
   #endif
   #if ENABLED(FASTER_GCODE_PARSER)
-    ZERO(codebits);                     // No codes yet
+    codebits = 0;                       // No codes yet
     //ZERO(param);                      // No parameters (should be safe to comment out this line)
   #endif
 }
@@ -196,14 +196,7 @@ void GCodeParser::parse(char *p) {
 
       while (*p == ' ') p++;                    // Skip spaces between parameters & values
 
-      const bool has_num = NUMERIC(p[0])                            // [0-9]
+      const bool has_num = valid_float(p);
-                        || (p[0] == '.' && NUMERIC(p[1]))           // .[0-9]
-                        || (
-                              (p[0] == '-' || p[0] == '+') && (     // [-+]
-                                NUMERIC(p[1])                       //     [0-9]
-                                || (p[1] == '.' && NUMERIC(p[2]))   //     .[0-9]
-                              )
-                            );
 
       #if ENABLED(DEBUG_GCODE_PARSER)
         if (debug) {
@@ -225,13 +218,7 @@ void GCodeParser::parse(char *p) {
       #endif
 
       #if ENABLED(FASTER_GCODE_PARSER)
-      {
+        set(code, has_num ? p : NULL);          // Set parameter exists and pointer (NULL for no number)
-        set(code, has_num ? p : NULL            // Set parameter exists and pointer (NULL for no number)
-          #if ENABLED(DEBUG_GCODE_PARSER)
-            , debug
-          #endif
-        );
-      }
       #endif
     }
     else if (!string_arg) {                     // Not A-Z? First time, keep as the string_arg

Marlin/src/gcode/parser.h

@@ -51,7 +51,7 @@ private:
   static char *value_ptr;           // Set by seen, used to fetch the value
 
   #if ENABLED(FASTER_GCODE_PARSER)
-    static byte codebits[4];        // Parameters pre-scanned
+    static uint32_t codebits;       // Parameters pre-scanned
     static uint8_t param[26];       // For A-Z, offsets into command args
   #else
     static char *command_args;      // Args start here, for slow scan
@@ -88,30 +88,34 @@ public:
   // Reset is done before parsing
   static void reset();
 
-  // Index so that 'X' falls on index 24
+  #define LETTER_BIT(N) ((N) - 'A')
-  #define PARAM_IND(N)  ((N) >> 3)
-  #define PARAM_BIT(N)  ((N) & 0x7)
-  #define LETTER_OFF(N) ((N) - 'A')
-  #define LETTER_IND(N) PARAM_IND(LETTER_OFF(N))
-  #define LETTER_BIT(N) PARAM_BIT(LETTER_OFF(N))
 
   #if ENABLED(FASTER_GCODE_PARSER)
 
+    FORCE_INLINE static bool valid_signless(const char * const p) {
+      return NUMERIC(p[0]) || (p[0] == '.' && NUMERIC(p[1])); // .?[0-9]
+    }
+
+    FORCE_INLINE static bool valid_float(const char * const p) {
+      return valid_signless(p) || ((p[0] == '-' || p[0] == '+') && valid_signless(&p[1])); // [-+]?.?[0-9]
+    }
+
+    FORCE_INLINE static bool valid_int(const char * const p) {
+      return NUMERIC(p[0]) || ((p[0] == '-' || p[0] == '+') && NUMERIC(p[1])); // [-+]?[0-9]
+    }
+
     // Set the flag and pointer for a parameter
-    static void set(const char c, char * const ptr
+    static void set(const char c, char * const ptr) {
-      #if ENABLED(DEBUG_GCODE_PARSER)
+      const uint8_t ind = LETTER_BIT(c);
-        , const bool debug=false
-      #endif
-    ) {
-      const uint8_t ind = LETTER_OFF(c);
       if (ind >= COUNT(param)) return;           // Only A-Z
-      SBI(codebits[PARAM_IND(ind)], PARAM_BIT(ind));        // parameter exists
+      SBI(codebits, ind);                        // parameter exists
       param[ind] = ptr ? ptr - command_ptr : 0;  // parameter offset or 0
       #if ENABLED(DEBUG_GCODE_PARSER)
-        if (debug) {
+        if (codenum == 800) {
-          SERIAL_ECHOPAIR("Set bit ", (int)PARAM_BIT(ind));
+          SERIAL_ECHOPAIR("Set bit ", (int)ind);
-          SERIAL_ECHOPAIR(" of index ", (int)PARAM_IND(ind));
+          SERIAL_ECHOPAIR(" of codebits (", hex_address((void*)(codebits >> 16)));
-          SERIAL_ECHOLNPAIR(" | param = ", (int)param[ind]);
+          print_hex_word((uint16_t)(codebits & 0xFFFF));
+          SERIAL_ECHOLNPAIR(") | param = ", (int)param[ind]);
         }
       #endif
     }
@@ -119,25 +123,28 @@ public:
     // Code seen bit was set. If not found, value_ptr is unchanged.
     // This allows "if (seen('A')||seen('B'))" to use the last-found value.
     static bool seen(const char c) {
-      const uint8_t ind = LETTER_OFF(c);
+      const uint8_t ind = LETTER_BIT(c);
       if (ind >= COUNT(param)) return false; // Only A-Z
-      const bool b = TEST(codebits[PARAM_IND(ind)], PARAM_BIT(ind));
+      const bool b = TEST(codebits, ind);
-      if (b) value_ptr = param[ind] ? command_ptr + param[ind] : (char*)NULL;
+      if (b) {
+        char * const ptr = command_ptr + param[ind];
+        value_ptr = param[ind] && valid_float(ptr) ? ptr : (char*)NULL;
+      }
       return b;
     }
 
-    static bool seen_any() { return codebits[3] || codebits[2] || codebits[1] || codebits[0]; }
+    static bool seen_any() { return !!codebits; }
 
-    #define SEEN_TEST(L) TEST(codebits[LETTER_IND(L)], LETTER_BIT(L))
+    #define SEEN_TEST(L) TEST(codebits, LETTER_BIT(L))
 
   #else // !FASTER_GCODE_PARSER
 
     // Code is found in the string. If not found, value_ptr is unchanged.
     // This allows "if (seen('A')||seen('B'))" to use the last-found value.
     static bool seen(const char c) {
-      char *p = strchr(command_args, c);
+      const char *p = strchr(command_args, c);
       const bool b = !!p;
-      if (b) value_ptr = DECIMAL_SIGNED(p[1]) ? &p[1] : (char*)NULL;
+      if (b) value_ptr = valid_float(&p[1]) ? &p[1] : (char*)NULL;
       return b;
     }
 
@@ -201,7 +208,7 @@ public:
   inline static uint8_t value_byte() { return (uint8_t)constrain(value_long(), 0, 255); }
 
   // Bool is true with no value or non-zero
-  inline static bool value_bool() { return !has_value() || value_byte(); }
+  inline static bool value_bool() { return !has_value() || !!value_byte(); }
 
   // Units modes: Inches, Fahrenheit, Kelvin
 
@@ -307,7 +314,6 @@ public:
   FORCE_INLINE static uint32_t ulongval(const char c, const uint32_t dval=0)  { return seenval(c) ? value_ulong()        : dval; }
   FORCE_INLINE static float    linearval(const char c, const float dval=0.0)  { return seenval(c) ? value_linear_units() : dval; }
   FORCE_INLINE static float    celsiusval(const char c, const float dval=0.0) { return seenval(c) ? value_celsius()      : dval; }
-  FORCE_INLINE static const char* strval(const char c)                           { return seenval(c) ? value_ptr            : NULL; }
 
 };
 

Marlin/src/module/configuration_store.cpp

@@ -1502,7 +1502,7 @@ void MarlinSettings::postprocess() {
         HAL::PersistentStore::access_finish();
 
         if (status)
-          SERIAL_PROTOCOL("?Unable to save mesh data.\n");
+          SERIAL_PROTOCOLPGM("?Unable to save mesh data.\n");
 
         // Write crc to MAT along with other data, or just tack on to the beginning or end
 
@@ -1540,7 +1540,7 @@ void MarlinSettings::postprocess() {
         HAL::PersistentStore::access_finish();
 
         if (status)
-          SERIAL_PROTOCOL("?Unable to load mesh data.\n");
+          SERIAL_PROTOCOLPGM("?Unable to load mesh data.\n");
 
         #if ENABLED(EEPROM_CHITCHAT)
           else

Marlin/src/module/temperature.cpp

@@ -266,11 +266,11 @@ uint8_t Temperature::soft_pwm_amount[HOTENDS],
     #endif
 
     if (!WITHIN(hotend, _BOT_HOTEND, _TOP_HOTEND)) {
-      SERIAL_ECHOLN(MSG_PID_BAD_EXTRUDER_NUM);
+      SERIAL_ECHOLNPGM(MSG_PID_BAD_EXTRUDER_NUM);
       return;
     }
 
-    SERIAL_ECHOLN(MSG_PID_AUTOTUNE_START);
+    SERIAL_ECHOLNPGM(MSG_PID_AUTOTUNE_START);
 
     disable_all_heaters(); // switch off all heaters.
 

Marlin/src/module/tool_change.cpp

@@ -123,7 +123,7 @@ inline void invalid_extruder_error(const uint8_t e) {
   SERIAL_CHAR('T');
   SERIAL_ECHO_F(e, DEC);
   SERIAL_CHAR(' ');
-  SERIAL_ECHOLN(MSG_INVALID_EXTRUDER);
+  SERIAL_ECHOLNPGM(MSG_INVALID_EXTRUDER);
 }
 
 /**

Marlin/src/sd/cardreader.cpp

@@ -364,7 +364,7 @@ void CardReader::openFile(char* name, const bool read, const bool subcall/*=fals
       if (file_subcall_ctr > SD_PROCEDURE_DEPTH - 1) {
         SERIAL_ERROR_START();
         SERIAL_ERRORPGM("trying to call sub-gcode files with too many levels. MAX level is:");
-        SERIAL_ERRORLN(SD_PROCEDURE_DEPTH);
+        SERIAL_ERRORLN((int)SD_PROCEDURE_DEPTH);
         kill(PSTR(MSG_KILLED));
         return;
       }