Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix logging exception which is thrown if command output contains non-ASCII characters #18

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

Kami
Copy link

@Kami Kami commented Apr 29, 2020

This pull request fixes an exception which was thrown when command produced output which contains non-ASCII characters.

Here is an example of the error before the fix:

Traceback (most recent call last):
  File "/home/pi/oprint/local/lib/python2.7/site-packages/octoprint/util/comm.py", line 3332, in _process_command_phase
    hook_results = hook(self, phase, command, command_type, gcode, subcode=subcode, tags=tags)
  File "/home/pi/oprint/local/lib/python2.7/site-packages/octoprint_gcodesystemcommands/__init__.py", line 79, in hook_gcode_sending
    self._logger.debug("Command ID %s returned: %s, output=%s" % (cmd_id, r, output))
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc2 in position 50: ordinal not in range(128)

And after:

2020-04-29 20:01:23,967 - octoprint.plugins.gcodesystemcommands - DEBUG - Command ID=907, Line=/home/pi/scripts/turn-off-printer.sh, Args=None
2020-04-29 20:01:23,976 - octoprint.plugins.gcodesystemcommands - INFO - Executing command ID: 907
2020-04-29 20:01:31,713 - octoprint.plugins.gcodesystemcommands - DEBUG - Command ID 907 returned: 0, output=Power: True
USB Power: None
Temperature: 46 °C
Load power: None
WiFi LED: None
Powering on
['ok']

2020-04-29 20:01:31,714 - octoprint.plugins.gcodesystemcommands - INFO - Command ID 907 returned: 0

I've tested it using Python 2.7 and Python 3.6.

@@ -1 +1,3 @@
OctoPrint
# six is already required by OctoPrint, but still add it here just in case
six
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In theory, this shouldn't be needed since OctoPrint already depends on it.

But it also doesn't hurt , especially since we don't pin it to a specific version so there is no chance of a version conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant