🐛 Defer M20 commands until initial capability report is done (#4577)

* Defer M20 commands until initial capability report is done * Fix whitepsace * Add setting for deferring sd file refresh * Refactor to be more similar to _firmware_info implementation * Add event hook for after capability report * Improve capability report hook name * Fix headline formatting Co-authored-by: Gina Häußge <[email protected]>
OctoPrint · Sep 28, 2022 · 84b83b6 · 84b83b6
1 parent cf25c52
commit 84b83b6
Show file tree

Hide file tree

Showing 7 changed files with 89 additions and 5 deletions.
diff --git a/docs/configuration/config_yaml.rst b/docs/configuration/config_yaml.rst
@@ -764,6 +764,10 @@ Use the following settings to configure the serial connection to the printer:
      # during connect.
      waitForStartOnConnect: false
 
+     # Specifies whether OctoPrint should wait to load the SD card file list until the first firmware capability
+     # report is processed.
+     waitToLoadSdFileList: false
+
      # Specifies whether OctoPrint should send linenumber + checksum with every printer command. Needed for
      # successful communication with Repetier firmware
      alwaysSendChecksum: false

diff --git a/docs/plugins/hooks.rst b/docs/plugins/hooks.rst
@@ -509,6 +509,30 @@ octoprint.comm.protocol.firmware.capabilities
 
 .. _sec-plugins-hook-comm-protocol-action:
 
+octoprint.comm.protocol.firmware.capability_report
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+.. py:function:: firmware_capability_report_hook(comm_instance, firmware_capabilities, *args, **kwargs)
+
+   .. versionadded:: 1.9.0
+
+   Be notified when all capability report entries are received from the printer.
+
+   Hook handlers may use this to react to the end of the custom firmware capability report. OctoPrint parses the received
+   capability lines and provides a dictionary of all reported capabilities and whether they're enabled to the handler.
+
+   .. warning::
+
+      Make sure to not perform any computationally expensive or otherwise long running actions within these handlers as
+      you will effectively block the receive loop, causing the communication with the printer to stall.
+
+      This includes I/O of any kind.
+
+   :param object comm_instance: The :class:`~octoprint.util.comm.MachineCom` instance which triggered the hook.
+   :param dict firmware_capabilities: Reported capabilities (capability name mapped to enabled flag)
+
+.. _sec-plugins-hook-comm-protocol-action:
+
 octoprint.comm.protocol.action
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

diff --git a/src/octoprint/schema/config/serial.py b/src/octoprint/schema/config/serial.py
@@ -167,6 +167,9 @@ class SerialConfig(BaseModel):
     waitForStartOnConnect: bool = False
     """Whether OctoPrint should wait for the `start` response from the printer before trying to send commands during connect."""
 
+    waitToLoadSdFileList: bool = False
+    """Specifies whether OctoPrint should wait to load the SD card file list until the first firmware capability report is processed."""
+
     alwaysSendChecksum: bool = False
     """Specifies whether OctoPrint should send linenumber + checksum with every printer command. Needed for successful communication with Repetier firmware."""
 

diff --git a/src/octoprint/server/api/settings.py b/src/octoprint/server/api/settings.py
@@ -213,6 +213,7 @@ def getSettings():
             "abortHeatupOnCancel": s.getBoolean(["serial", "abortHeatupOnCancel"]),
             "supportResendsWithoutOk": s.get(["serial", "supportResendsWithoutOk"]),
             "waitForStart": s.getBoolean(["serial", "waitForStartOnConnect"]),
+            "waitToLoadSdFileList": s.getBoolean(["serial", "waitToLoadSdFileList"]),
             "alwaysSendChecksum": s.getBoolean(["serial", "alwaysSendChecksum"]),
             "neverSendChecksum": s.getBoolean(["serial", "neverSendChecksum"]),
             "sendChecksumWithUnknownCommands": s.getBoolean(
@@ -834,6 +835,11 @@ def _saveSettings(data):
             s.setBoolean(
                 ["serial", "waitForStartOnConnect"], data["serial"]["waitForStart"]
             )
+        if "waitToLoadSdFileList" in data["serial"]:
+            s.setBoolean(
+                ["serial", "waitToLoadSdFileList"],
+                data["serial"]["waitToLoadSdFileList"],
+            )
         if "alwaysSendChecksum" in data["serial"]:
             s.setBoolean(
                 ["serial", "alwaysSendChecksum"], data["serial"]["alwaysSendChecksum"]

diff --git a/src/octoprint/static/js/app/viewmodels/settings.js b/src/octoprint/static/js/app/viewmodels/settings.js
@@ -210,6 +210,7 @@ $(function () {
         self.serial_serialErrorBehaviour = ko.observable("cancel");
         self.serial_triggerOkForM29 = ko.observable(undefined);
         self.serial_waitForStart = ko.observable(undefined);
+        self.serial_waitToLoadSdFileList = ko.observable(undefined);
         self.serial_sendChecksum = ko.observable("print");
         self.serial_sendChecksumWithUnknownCommands = ko.observable(undefined);
         self.serial_unknownCommandsNeedAck = ko.observable(undefined);
@@ -392,6 +393,7 @@ $(function () {
 
         self.observableCopies = {
             feature_waitForStart: "serial_waitForStart",
+            feature_waitToLoadSdFileList: "serial_waitToLoadSdFileList",
             feature_sendChecksum: "serial_sendChecksum",
             feature_sdRelativePath: "serial_sdRelativePath",
             feature_sdAlwaysAvailable: "serial_sdAlwaysAvailable",

diff --git a/src/octoprint/templates/dialogs/settings/serialconnection.jinja2 b/src/octoprint/templates/dialogs/settings/serialconnection.jinja2
@@ -453,6 +453,12 @@
                             <span class="help-block">{{ _('Try checking this if you run into connection timeouts due to your printer taking too long to respond to OctoPrint\'s handshake attempts on connect.') }}</span>
                         </label>
                     </div>
+                    <div class="controls">
+                        <label class="checkbox">
+                            <input type="checkbox" data-bind="checked: serial_waitToLoadSdFileList" id="settings-serialWaitToLoadSdFileList"> {{ _('Wait to load the SD card file list until the firmware capability report is processed.') }}
+                            <span class="help-block">{{ _('Try checking this if the SD card file list loads improperly after connecting to the printer.') }}</span>
+                        </label>
+                    </div>
                 </div>
                 <div class="control-group">
                     <label class="control-label" for="settings-serialBlockedCommands">{{ _('Blocked commands') }}</label>

diff --git a/src/octoprint/util/comm.py b/src/octoprint/util/comm.py
@@ -622,9 +622,12 @@ def __init__(
 
         self._firmware_detection = settings().getBoolean(["serial", "firmwareDetection"])
         self._firmware_info_received = False
+        self._firmware_capabilities_received = False
         self._firmware_info = {}
         self._firmware_capabilities = {}
 
+        self._defer_sd_refresh = settings().getBoolean(["serial", "waitToLoadSdFileList"])
+
         self._temperature_autoreporting = False
         self._sdstatus_autoreporting = False
         self._pos_autoreporting = False
@@ -725,6 +728,9 @@ def __init__(
             "capabilities": self._pluginManager.get_hooks(
                 "octoprint.comm.protocol.firmware.capabilities"
             ),
+            "capability_report": self._pluginManager.get_hooks(
+                "octoprint.comm.protocol.firmware.capability_report"
+            ),
         }
 
         self._printer_action_hooks = self._pluginManager.get_hooks(
@@ -2004,6 +2010,12 @@ def refreshSdFiles(self, tags=None):
         if not self.isOperational() or self.isBusy():
             return
 
+        if self._defer_sd_refresh and not self._firmware_capabilities_received:
+            self._logger.debug(
+                "Deferring sd file refresh until capability report is processed"
+            )
+            return
+
         if tags is None:
             tags = set()
 
@@ -2558,6 +2570,33 @@ def convert_line(line):
                 ):
                     continue
 
+                # wait for the end of the firmware capability report (M115) then notify plugins and refresh sd list if deferred
+                if (
+                    self._firmware_capabilities
+                    and not self._firmware_capabilities_received
+                    and not lower_line.startswith("cap:")
+                ):
+                    self._firmware_capabilities_received = True
+
+                    if self._defer_sd_refresh:
+                        # sd list was deferred, refresh it now
+                        self._logger.debug("Performing deferred sd file refresh")
+                        self.refreshSdFiles()
+
+                    # notify plugins
+                    for name, hook in self._firmware_info_hooks[
+                        "capability_report"
+                    ].items():
+                        try:
+                            hook(self, copy.copy(self._firmware_capabilities))
+                        except Exception:
+                            self._logger.exception(
+                                "Error processing firmware reported hook {}:".format(
+                                    name
+                                ),
+                                extra={"plugin": name},
+                            )
+
                 ##~~ position report processing
                 if "X:" in line and "Y:" in line and "Z:" in line:
                     parsed = parse_position_line(line)
@@ -2707,6 +2746,10 @@ def convert_line(line):
                                 self._logger.info(
                                     "Firmware states that it supports emergency GCODEs to be sent without waiting for an acknowledgement first"
                                 )
+                            elif capability == self.CAPABILITY_EXTENDED_M20 and enabled:
+                                self._logger.info(
+                                    "Firmware states that it supports long filenames"
+                                )
 
                         # notify plugins
                         for name, hook in self._firmware_info_hooks[
@@ -3605,11 +3648,7 @@ def _onConnected(self):
         )
 
         if self._sdAvailable:
-            self.refreshSdFiles(
-                tags={
-                    "trigger:comm.on_connected",
-                }
-            )
+            self.refreshSdFiles(tags={"trigger:comm.on_connected"})
         else:
             self.initSdCard(tags={"trigger:comm.on_connected"})