gcode: Improve handling of extended g-code commands with '*;#' charac…

…ters (#450)

* gcode: Validate extended g-code command names

Extended g-code command names may only contain A-Z, 0-9, and
underscore, and the first two characters may not be digits.

Signed-off-by: Kevin O'Connor <[email protected]>

* gcode: Don't silently discard characters inside a command name

Don't silently drop leading numbers and unusual characters at the
start of a command - for example, don't interpret '99M88' as 'M88'.

Don't silently drop spaces in a command - for example, don't interpret
"M 101" as the command "M101".  Doing so will cause other parts of the
code (such as get_raw_command_parameters() ) to not work properly.

Signed-off-by: Kevin O'Connor <[email protected]>

* gcode: Improve handling of extended g-code commands with '*;#' characters

The g-code command parser did not allow three characters to be passed
as parameters to commands (asterisk, semicolon, pound sign).  Rework
the parsing code to better leverage the python shlex package so that
these characters can be supported.

In particular, this should allow better support for printing g-code
files that have unusual characters in the filename.

Signed-off-by: Kevin O'Connor <[email protected]>

* gcode: Fixup M117/M118 command identification in cmd_default()

Alter gcmd._command in cmd_default if the special M117/M118 handling
is detected.  This avoids having to recheck for this condition in
get_raw_command_parameters().

Signed-off-by: Kevin O'Connor <[email protected]>

* gcode: Use the same M117/M118 fixup for M23

The M23 command has similar requirements for extracting the full
parameter string that M117/M118 have.  Use the same code for those
fixups.

Signed-off-by: Kevin O'Connor <[email protected]>

* gcode: Some optimizations to get_raw_command_parameters()

Add some minor optimizations to the get_raw_command_parameters() code.

Signed-off-by: Kevin O'Connor <[email protected]>

* gcode: Improve checksum detection in get_raw_command_parameters()

Only consider a trailing '*' to indicate a checksum if the remainder
of the string is a number.

Signed-off-by: Kevin O'Connor <[email protected]>

---------

Signed-off-by: Kevin O'Connor <[email protected]>
Co-authored-by: Kevin O'Connor <[email protected]>

docs/Config_Changes.md

@@ -10,6 +10,12 @@ All dates in this document are approximate.
 
 20241202: The `sense_resistor` parameter is now mandatory with no default value.
 
+20241201: In some cases Klipper may have ignored leading characters or
+spaces in a traditional G-Code command. For example, "99M123" may have
+been interpreted as "M123" and "M 321" may have been interpreted as
+"M321". Klipper will now report these cases with an "Unknown command"
+warning.
+
 20241125: The `off_below` parameter in fans config section is
 deprecated. It will be removed in the near future. Use
 [`min_power`](./Config_Reference.md#fans)

klippy/gcode.py

@@ -1,6 +1,6 @@
 # Parse gcode commands
 #
-# Copyright (C) 2016-2021  Kevin O'Connor <[email protected]>
+# Copyright (C) 2016-2024  Kevin O'Connor <[email protected]>
 #
 # This file may be distributed under the terms of the GNU GPLv3 license.
 import os, re, logging, collections, shlex
@@ -36,19 +36,18 @@ def get_command_parameters(self):
 
     def get_raw_command_parameters(self):
         command = self._command
-        if command.startswith("M117 ") or command.startswith("M118 "):
-            command = command[:4]
-        rawparams = self._commandline
-        urawparams = rawparams.upper()
-        if not urawparams.startswith(command):
-            rawparams = rawparams[urawparams.find(command) :]
-            end = rawparams.rfind("*")
-            if end >= 0:
-                rawparams = rawparams[:end]
-        rawparams = rawparams[len(command) :]
-        if rawparams.startswith(" "):
-            rawparams = rawparams[1:]
-        return rawparams
+        origline = self._commandline
+        param_start = len(command)
+        param_end = len(origline)
+        if origline[:param_start].upper() != command:
+            # Skip any gcode line-number and ignore any trailing checksum
+            param_start += origline.upper().find(command)
+            end = origline.rfind("*")
+            if end >= 0 and origline[end + 1 :].isdigit():
+                param_end = end
+        if origline[param_start : param_start + 1].isspace():
+            param_start += 1
+        return origline[param_start:param_end]
 
     def ack(self, msg=None):
         if not self._need_ack:
@@ -203,6 +202,15 @@ def register_command(self, cmd, func, when_not_ready=False, desc=None):
                 "gcode command %s already registered" % (cmd,)
             )
         if not self.is_traditional_gcode(cmd):
+            if (
+                cmd.upper() != cmd
+                or not cmd.replace("_", "A").isalnum()
+                or cmd[0].isdigit()
+                or cmd[1:2].isdigit()
+            ):
+                raise self.printer.config_error(
+                    "Can't register '%s' as it is an invalid name" % (cmd,)
+                )
             origfunc = func
 
             def func(params):
@@ -271,7 +279,7 @@ def _handle_ready(self):
         self._respond_state("Ready")
 
     # Parse input into commands
-    args_r = re.compile("([A-Z_]+|[A-Z*/])")
+    args_r = re.compile("([A-Z_]+|[A-Z*])")
 
     def _process_commands(self, commands, need_ack=True):
         for line in commands:
@@ -282,16 +290,14 @@ def _process_commands(self, commands, need_ack=True):
                 line = line[:cpos]
             # Break line into parts and determine command
             parts = self.args_r.split(line.upper())
-            numparts = len(parts)
-            cmd = ""
-            if numparts >= 3 and parts[1] != "N":
-                cmd = parts[1] + parts[2].strip()
-            elif numparts >= 5 and parts[1] == "N":
+            if "".join(parts[:2]) == "N":
                 # Skip line number at start of command
-                cmd = parts[3] + parts[4].strip()
+                cmd = "".join(parts[3:5]).strip()
+            else:
+                cmd = "".join(parts[:3]).strip()
             # Build gcode "params" dictionary
             params = {
-                parts[i]: parts[i + 1].strip() for i in range(1, numparts, 2)
+                parts[i]: parts[i + 1].strip() for i in range(1, len(parts), 2)
             }
             gcmd = GCodeCommand(self, cmd, origline, params, need_ack)
             # Invoke handler for command
@@ -352,30 +358,23 @@ def _respond_state(self, state):
         self.respond_info("Klipper state: %s" % (state,), log=False)
 
     # Parameter parsing helpers
-    extended_r = re.compile(
-        r"^\s*(?:N[0-9]+\s*)?"
-        r"(?P<cmd>[a-zA-Z_][a-zA-Z0-9_]+)(?:\s+|$)"
-        r"(?P<args>[^#*;]*?)"
-        r"\s*(?:[#*;].*)?$"
-    )
-
     def _get_extended_params(self, gcmd):
-        m = self.extended_r.match(gcmd.get_commandline())
-        if m is None:
-            raise self.error(
-                "Malformed command '%s'" % (gcmd.get_commandline(),)
-            )
-        eargs = m.group("args")
+        rawparams = gcmd.get_raw_command_parameters()
+        # Extract args while allowing shell style quoting
+        s = shlex.shlex(rawparams, posix=True)
+        s.whitespace_split = True
+        s.commenters = "#;"
         try:
-            eparams = [earg.split("=", 1) for earg in shlex.split(eargs)]
+            eparams = [earg.split("=", 1) for earg in s]
             eparams = {k.upper(): v for k, v in eparams}
-            gcmd._params.clear()
-            gcmd._params.update(eparams)
-            return gcmd
         except ValueError as e:
             raise self.error(
                 "Malformed command '%s'" % (gcmd.get_commandline(),)
             )
+        # Update gcmd with new parameters
+        gcmd._params.clear()
+        gcmd._params.update(eparams)
+        return gcmd
 
     # G-Code special command handlers
     def cmd_default(self, gcmd):
@@ -395,12 +394,15 @@ def cmd_default(self, gcmd):
             if cmdline:
                 logging.debug(cmdline)
             return
-        if cmd.startswith("M117 ") or cmd.startswith("M118 "):
+        if " " in cmd:
             # Handle M117/M118 gcode with numeric and special characters
-            handler = self.gcode_handlers.get(cmd[:4], None)
-            if handler is not None:
-                handler(gcmd)
-                return
+            realcmd = cmd.split()[0]
+            if realcmd in ["M117", "M118", "M23"]:
+                handler = self.gcode_handlers.get(realcmd, None)
+                if handler is not None:
+                    gcmd._command = realcmd
+                    handler(gcmd)
+                    return
         elif cmd in ["M140", "M104"] and not gcmd.get_float("S", 0.0):
             # Don't warn about requests to turn off heaters when not present
             return