From e553a69ff5c9ecd8ed8b90fa58486bdce8e4b962 Mon Sep 17 00:00:00 2001 From: Alice Grey Date: Mon, 7 Sep 2020 11:37:01 -0500 Subject: [PATCH 1/2] Implement Configpath and Logpath Validation Added JS popup handler. Added configpath and logpath validation when settings are saved. Add logpath validation Change file checking method Check new values instead of old settings on save --- octoprint_klipper/__init__.py | 57 ++++-- octoprint_klipper/static/js/klipper.js | 256 +++++++++++++------------ 2 files changed, 179 insertions(+), 134 deletions(-) diff --git a/octoprint_klipper/__init__.py b/octoprint_klipper/__init__.py index d24b3ef..2a0d216 100644 --- a/octoprint_klipper/__init__.py +++ b/octoprint_klipper/__init__.py @@ -81,41 +81,55 @@ class KlipperPlugin( def on_settings_load(self): data = octoprint.plugin.SettingsPlugin.on_settings_load(self) - filepath = os.path.expanduser( + configpath = os.path.expanduser( self._settings.get(["configuration", "configpath"]) ) + + logpath = os.path.expanduser( + self._settings.get(["configuration", "logpath"]) + ) + try: - f = open(filepath, "r") + f = open(configpath, "r") data["config"] = f.read() f.close() except IOError: self._logger.error( - "Error: Klipper config file not found at: {}".format(filepath) + "Error: Klipper config file not found at: {}".format(configpath) ) return data def on_settings_save(self, data): + if self.keyexists(data, "configuration", "configpath"): + configpath = os.path.expanduser( + data["configuration"]["configpath"] + ) + self.checkFile("config",configpath) + + if self.keyexists(data, "configuration", "logpath"): + logpath = os.path.expanduser( + data["configuration"]["logpath"] + ) + self.checkFile("log",logpath) + if "config" in data: try: - filepath = os.path.expanduser( - self._settings.get(["configuration", "configpath"]) - ) data["config"] = data["config"].encode('utf-8') - f = open(filepath, "w") + f = open(configpath, "w") f.write(data["config"]) f.close() - self._logger.info( - "Writing Klipper config to {}".format(filepath) + self._logger.error( + "Writing Klipper config to {}".format(configpath) ) # Restart klipply to reload config self._printer.commands(self._settings.get(["configuration", "reload_command"])) self.logInfo("Reloading Klipper Configuration.") except IOError: self._logger.error( - "Error: Couldn't write Klipper config file: {}".format(filepath) + "Error: Couldn't write Klipper config file: {}".format(configpath) ) - data.pop("config", None) # we dont want to write the klipper conf to the octoprint settings + data.pop("config", None) # we dont want to write the klipper conf to the octoprint settings else: octoprint.plugin.SettingsPlugin.on_settings_save(self, data) @@ -324,13 +338,14 @@ class KlipperPlugin( ) #-- Helpers - def sendMessage(self, type, subtype, payload): + def sendMessage(self, type, subtype, title, payload): self._plugin_manager.send_plugin_message( self._identifier, dict( time=datetime.datetime.now().strftime("%H:%M:%S"), type=type, subtype=subtype, + title=title, payload=payload ) ) @@ -339,13 +354,25 @@ class KlipperPlugin( self._printer.commands("STATUS") def updateStatus(self, type, status): - self.sendMessage("status", type, status) + self.sendMessage("status", type, status, status) def logInfo(self, message): - self.sendMessage("log", "info", message) + self.sendMessage("log", "info", message, message) def logError(self, error): - self.sendMessage("log", "error", error) + self.sendMessage("log", "error", error, error) + + def checkFile(self,filetype,filepath): + if not os.path.isfile(filepath): + self.sendMessage("errorPopUp","warning", "OctoKlipper Settings", "Klipper " + filepath + " does not exist!") + + def keyexists(self, dict, key1, key2): + try: + dict[key1][key2] + except KeyError: + return False + else: + return True __plugin_name__ = "OctoKlipper" __plugin_pythoncompat__ = ">=2.7,<4" diff --git a/octoprint_klipper/static/js/klipper.js b/octoprint_klipper/static/js/klipper.js index 6335409..8d9d1ff 100644 --- a/octoprint_klipper/static/js/klipper.js +++ b/octoprint_klipper/static/js/klipper.js @@ -14,126 +14,144 @@ // along with this program. If not, see . $(function() { - function KlipperViewModel(parameters) { - var self = this; + function KlipperViewModel(parameters) { + var self = this; - self.settings = parameters[0]; - self.loginState = parameters[1]; - self.connectionState = parameters[2]; - self.levelingViewModel = parameters[3]; - self.paramMacroViewModel = parameters[4]; - - self.shortStatus = ko.observable(); - self.logMessages = ko.observableArray(); - - self.showLevelingDialog = function() { - var dialog = $("#klipper_leveling_dialog"); - dialog.modal({ - show: 'true', - backdrop: 'static', - keyboard: false - }); - self.levelingViewModel.initView(); - } - - self.showPidTuningDialog = function() { - var dialog = $("#klipper_pid_tuning_dialog"); - dialog.modal({ - show: 'true', - backdrop: 'static', - keyboard: false - }); - } - - self.showOffsetDialog = function() { - var dialog = $("#klipper_offset_dialog"); - dialog.modal({ - show: 'true', - backdrop: 'static' - }); - } - - self.showGraphDialog = function() { - var dialog = $("#klipper_graph_dialog"); - dialog.modal({ - show: 'true', - minHeight: "500px", - maxHeight: "600px" - }); - } - - self.executeMacro = function(macro) { - var paramObjRegex = /{(.*?)}/g; - - if (macro.macro().match(paramObjRegex) == null) { - OctoPrint.control.sendGcode( - // Use .split to create an array of strings which is sent to - // OctoPrint.control.sendGcode instead of a single string. - macro.macro().split(/\r\n|\r|\n/) - ); - } else { - self.paramMacroViewModel.process(macro); - - var dialog = $("#klipper_macro_dialog"); - dialog.modal({ - show: 'true', - backdrop: 'static' - }); - } - } - - self.onGetStatus = function() { - OctoPrint.control.sendGcode("Status") - } - - self.onRestartFirmware = function() { - OctoPrint.control.sendGcode("FIRMWARE_RESTART") - }; - - self.onRestartHost = function() { - OctoPrint.control.sendGcode("RESTART") - }; - - self.onAfterBinding = function() { - self.connectionState.selectedPort(self.settings.settings.plugins.klipper.connection.port()); - } - - self.onDataUpdaterPluginMessage = function(plugin, message) { - if(plugin == "klipper") { - if(message["type"] == "status") { - self.shortStatus(message["payload"]); - } else { - self.logMessage(message["time"], message["subtype"], message["payload"]); - } - } - } + self.settings = parameters[0]; + self.loginState = parameters[1]; + self.connectionState = parameters[2]; + self.levelingViewModel = parameters[3]; + self.paramMacroViewModel = parameters[4]; + + self.shortStatus = ko.observable(); + self.logMessages = ko.observableArray(); - self.logMessage = function(timestamp, type, message) { - self.logMessages.push({ - time: timestamp, - type: type, - msg: message.replace(/\n/gi, "
")} - ); - } - - self.onClearLog = function() { - self.logMessages.removeAll(); - }; - - self.isActive = function() { - return self.connectionState.isOperational() && self.loginState.isUser(); - } - } + self.showPopUp = function(popupType, popupTitle, message){ + var title = popupType.toUpperCase() + ": " + popupTitle; + new PNotify({ + title: title, + text: message, + type: popupType, + hide: false + }); + }; + + self.showLevelingDialog = function() { + var dialog = $("#klipper_leveling_dialog"); + dialog.modal({ + show: 'true', + backdrop: 'static', + keyboard: false + }); + self.levelingViewModel.initView(); + }; + + self.showPidTuningDialog = function() { + var dialog = $("#klipper_pid_tuning_dialog"); + dialog.modal({ + show: 'true', + backdrop: 'static', + keyboard: false + }); + }; + + self.showOffsetDialog = function() { + var dialog = $("#klipper_offset_dialog"); + dialog.modal({ + show: 'true', + backdrop: 'static' + }); + }; + + self.showGraphDialog = function() { + var dialog = $("#klipper_graph_dialog"); + dialog.modal({ + show: 'true', + minHeight: "500px", + maxHeight: "600px" + }); + }; + + self.executeMacro = function(macro) { + var paramObjRegex = /{(.*?)}/g; + + if (macro.macro().match(paramObjRegex) == null) { + OctoPrint.control.sendGcode( + // Use .split to create an array of strings which is sent to + // OctoPrint.control.sendGcode instead of a single string. + macro.macro().split(/\r\n|\r|\n/) + ); + } else { + self.paramMacroViewModel.process(macro); + + var dialog = $("#klipper_macro_dialog"); + dialog.modal({ + show: 'true', + backdrop: 'static' + }); + } + }; + + self.onGetStatus = function() { + OctoPrint.control.sendGcode("Status") + }; + + self.onRestartFirmware = function() { + OctoPrint.control.sendGcode("FIRMWARE_RESTART") + }; + + self.onRestartHost = function() { + OctoPrint.control.sendGcode("RESTART") + }; + + self.onAfterBinding = function() { + self.connectionState.selectedPort(self.settings.settings.plugins.klipper.connection.port()); + }; + + self.onDataUpdaterPluginMessage = function(plugin, data) { + if(plugin == "klipper") { + if ("warningPopUp" == data.type){ + self.showPopUp(data.subtype, data.title, data.payload); + return; + } + if ("errorPopUp" == data.type){ + self.showPopUp(data.subtype, data.title, data.payload); + return; + } + if(data.type == "status") { + self.shortStatus(data.payload); + } else { + self.logMessage(data.time, data.subtype, data.payload); + } + } + }; - OCTOPRINT_VIEWMODELS.push({ - construct: KlipperViewModel, - dependencies: [ - "settingsViewModel", - "loginStateViewModel", - "connectionViewModel", - "klipperLevelingViewModel", - "klipperMacroDialogViewModel" - ], - elements: ["#tab_plugin_klipper_main", "#sidebar_plugin_klipper", "#navbar_plugin_klipper"] - }); + self.logMessage = function(timestamp, type, message) { + self.logMessages.push({ + time: timestamp, + type: type, + msg: message.replace(/\n/gi, "
")} + ); + }; + + self.onClearLog = function() { + self.logMessages.removeAll(); + }; + + self.isActive = function() { + return self.connectionState.isOperational() && self.loginState.isUser(); + }; + } + + OCTOPRINT_VIEWMODELS.push({ + construct: KlipperViewModel, + dependencies: [ + "settingsViewModel", + "loginStateViewModel", + "connectionViewModel", + "klipperLevelingViewModel", + "klipperMacroDialogViewModel" + ], + elements: ["#tab_plugin_klipper_main", "#sidebar_plugin_klipper", "#navbar_plugin_klipper"] + }); }); From 1d8a6cb35296cd6415ab317f2f31ad1a885f6c77 Mon Sep 17 00:00:00 2001 From: Oliver Fawcett-Griffiths Date: Sun, 4 Oct 2020 16:31:10 +1300 Subject: [PATCH 2/2] Implement further config validation Basically just parse the config file before we save it, ensuring that it can be parsed by `configparser` - the same parser used by Klipper. This PR depende on the changes from https://github.com/AliceGrey/OctoprintKlipperPlugin/pull/8 in order to send a warning popup when validation fails. --- octoprint_klipper/__init__.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/octoprint_klipper/__init__.py b/octoprint_klipper/__init__.py index 2a0d216..fc35621 100644 --- a/octoprint_klipper/__init__.py +++ b/octoprint_klipper/__init__.py @@ -22,6 +22,7 @@ import os from octoprint.util.comm import parse_firmware_line from .modules import KlipperLogAnalyzer import flask +import configparser class KlipperPlugin( octoprint.plugin.StartupPlugin, @@ -116,6 +117,17 @@ class KlipperPlugin( try: data["config"] = data["config"].encode('utf-8') + # Basic validation of config file - makes sure it parses + try: + parser = configparser.RawConfigParser() + parser.read_string(data["config"]) + except configParser.Error as error: + self._logger.error( + "Error: Invalid Klipper config file: {}".format(str(error)) + ) + self.sendMessage("errorPopUp","warning", "OctoKlipper Settings", "Invalid Klipper config file: " + str(error)) + + f = open(configpath, "w") f.write(data["config"]) f.close()