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

Python3 support #3278

Merged
merged 21 commits into from
Oct 6, 2021
Merged

Python3 support #3278

merged 21 commits into from
Oct 6, 2021

Conversation

KevinOConnor
Copy link
Collaborator

@KevinOConnor KevinOConnor commented Sep 3, 2020

This branch tracks changes to the Klipper host code so that it can run on Python3. It is intended for developers that wish to test and develop Klipper on Python3.

There are no immediate plans to switch Klipper from Python2 to Python3.

For those that do wish to test, it will require pulling this branch, installing a python3 virtualenv, and running the new code. For example:

sudo service klipper stop
cd ~/klipper ; git fetch ; git checkout origin/work-python3-20200612
virtualenv -p python3 ~/python3-env
~/python3-env/bin/pip install -r ~/klipper/scripts/klippy-requirements.txt

~/python3-env/bin/python ~/klipper/klippy/klippy.py ~/printer.cfg -l /tmp/klippy.log

At a minimum, Python version 3.6 or later is necessary.

If during testing you find an issue, please report as a comment on this PR. Code fixes for Python3 should be submitted as PRs against this branch (work-python3-20200620).

-Kevin

P.S. Please don't comment here requesting we convert to Python3. Please don't comment here to inform us of the pending doom of Python2. We've known about the state of Python2 for over a decade. We'll switch to Python3 if and when we feel that is appropriate - it will not be based on the schedules of any other projects.

@nadvornik
Copy link

Hi,
I am trying to install Klipper on Armbian.
Python2 failed on dependencies, then I found this branch. It mostly works, but I got these errors:

Error during display screen update
Traceback (most recent call last):
  File "/home/vn/klipper/klippy/extras/display/display.py", line 208, in screen_update_event
    self.show_data_group.show(self, self.display_templates, eventtime)
  File "/home/vn/klipper/klippy/extras/display/display.py", line 81, in show
    display.draw_text(row, col, text.replace('\n', ''), eventtime)
  File "/home/vn/klipper/klippy/extras/display/display.py", line 223, in draw_text
    self.lcd_chip.write_text(pos, row, text)
  File "/home/vn/klipper/klippy/extras/display/st7920.py", line 137, in write_text
    self.text_framebuffer[pos:pos+len(data)] = data
TypeError: can assign only bytes, buffers, or iterables of ints in range(0, 256)
Stats 542.5: gcodein=0 mcu: mcu_awake=0.000 mcu_task_avg=0.000000 mcu_task_stddev=0.000000 bytes_write=1528 bytes_read=3418 bytes_retransmit=9 bytes_invalid=6 send_seq=115 receive_seq=112 retransmit_seq=2 srtt=0
.004 rttvar=0.001 rto=0.025 ready_bytes=1339 stalled_bytes=0 freq=15998565 heater_bed: target=0 temp=0.0 pwm=0.000  print_time=0.001 buffer_time=0.000 print_stall=0 extruder: target=0 temp=0.0 pwm=0.000 sysload=
0.24 cputime=2.660
Exception in serial callback
Traceback (most recent call last):
  File "/home/vn/klipper/klippy/serialhdl.py", line 59, in _bg_thread
    hdl(params)
  File "/home/vn/klipper/klippy/extras/buttons.py", line 76, in handle_buttons_state
    (lambda e, s=self, b=ord(b): s.handle_button(e, b)))
TypeError: ord() expected string of length 1, but int found

@nadvornik
Copy link

One more:

Internal error on command:"PID_CALIBRATE"
Traceback (most recent call last):
  File "/home/vn/klipper/klippy/gcode.py", line 177, in _process_commands
    handler(gcmd)
  File "/home/vn/klipper/klippy/gcode.py", line 115, in <lambda>
    func = lambda params: origfunc(self._get_extended_params(params))
  File "/home/vn/klipper/klippy/extras/pid_calibrate.py", line 40, in cmd_PID_CALIBRATE
    Kp, Ki, Kd = calibrate.calc_final_pid()
  File "/home/vn/klipper/klippy/extras/pid_calibrate.py", line 133, in calc_final_pid
    midpoint_pos = sorted(cycle_times)[len(cycle_times)/2][1]
TypeError: list indices must be integers or slices, not float
Transition to shutdown state: Internal error on command:"PID_CALIBRATE"

@KevinOConnor
Copy link
Collaborator Author

Thanks. I think those two errors should be fixed now. (Follow the "git fetch ; git checkout" commands above to pull the latest code.)

Separately, if you just want to get printing, it will probably be easier to install python2. Otherwise, feel free to continue testing.

-Kevin

@KevinOConnor KevinOConnor force-pushed the work-python3-20200612 branch 2 times, most recently from 8e4fadf to b55c594 Compare September 5, 2020 02:32
@nadvornik
Copy link

It worked good enough to successfully print several models, then after restart I got this:

Starting SD card print (position 0)
Unhandled exception during run
Traceback (most recent call last):
  File "/home/vn/klipper/klippy/klippy.py", line 188, in run
    self.reactor.run()
  File "/home/vn/klipper/klippy/reactor.py", line 251, in run
    g_next.switch()
  File "/home/vn/klipper/klippy/reactor.py", line 278, in _dispatch_loop
    timeout = self._check_timers(eventtime)
  File "/home/vn/klipper/klippy/reactor.py", line 136, in _check_timers
    t.waketime = waketime = t.callback(eventtime)
  File "/home/vn/klipper/klippy/extras/virtual_sdcard.py", line 218, in work_handler
    lines = data.split('\n')
TypeError: a bytes-like object is required, not 'str'

@KevinOConnor
Copy link
Collaborator Author

Should be fixed now.

-Kevin

@cbc02009
Copy link
Contributor

cbc02009 commented Oct 3, 2020

Not sure if this was already known or not but buildcommands.py is not python3 compatible in this branch, so the firmware for the controlled boards must be built on a separate device that has python2 support.

@cbc02009
Copy link
Contributor

cbc02009 commented Oct 5, 2020

Can't get klippy to communicate with my BTT SKR 1.4s using this branch. Getting a command format mismatch error. Tested exact same firmware on master branch without issue.

Here's the klippy.log:
klippy.log

@KevinOConnor
Copy link
Collaborator Author

Not sure if this was already known or not but buildcommands.py is not python3 compatible in this branch

I updated this branch to also change buildcommands.py to Python3.

Can't get klippy to communicate with my BTT SKR 1.4s using this branch. Getting a command format mismatch error. Tested exact same firmware on master branch without issue.

This branch isn't always kept in sync with the master branch, so in general, it will be necessary to compile and flash the micro-controller using code on this branch. (That is, it may not be possible to use the micro-controller code from the master branch with the host code from this branch.) That said, I just rebase'd this branch with the latest master branch, so there should no longer be a conflict right now.

-Kevin

@maverich
Copy link

Hi,
I cannot start klippy with -a /tmp/klippy_uds but without it everything looks fine
Logs: klippy.log

@KevinOConnor
Copy link
Collaborator Author

Thanks. I made some additional changes for webhooks python3 support.

-Kevin

@ssorgatem
Copy link

With this branch klipper cannot connect to a linux-process MCU: "Unable to open port: File or stream is not seekable."

Apparently it's the same issue as here: stefanholek/term#1 , with hopefully an easy fix.

@abduct
Copy link

abduct commented Nov 13, 2020

Another webhooks error:

webhooks client 3039482664: New connection
webhooks client 3039482664: Client info {'program': 'Moonraker', 'version': 'v0.1.0-101-g697be6a'}
Unhandled exception during run
Traceback (most recent call last):
  File "/home/printer/klipper/klippy/klippy.py", line 188, in run
    self.reactor.run()
  File "/home/printer/klipper/klippy/reactor.py", line 269, in run
    g_next.switch()
  File "/home/printer/klipper/klippy/reactor.py", line 310, in _dispatch_loop
    timeout = self._check_timers(eventtime, busy)
  File "/home/printer/klipper/klippy/reactor.py", line 156, in _check_timers
    t.waketime = waketime = t.callback(eventtime)
  File "/home/printer/klipper/klippy/reactor.py", line 48, in invoke
    res = self.callback(eventtime)
  File "/home/printer/klipper/klippy/webhooks.py", line 225, in <lambda>
    lambda e, s=self, wr=web_request: s._process_request(wr))
  File "/home/printer/klipper/klippy/webhooks.py", line 242, in _process_request
    self.send(result)
  File "/home/printer/klipper/klippy/webhooks.py", line 245, in send
    self.send_buffer += json.dumps(data).encode() + b"\x03"
  File "/usr/lib/python3.8/json/__init__.py", line 231, in dumps
    return _default_encoder.encode(obj)
  File "/usr/lib/python3.8/json/encoder.py", line 199, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/usr/lib/python3.8/json/encoder.py", line 257, in iterencode
    return _iterencode(o, 0)
  File "/usr/lib/python3.8/json/encoder.py", line 179, in default
    raise TypeError(f'Object of type {o.__class__.__name__} '
TypeError: Object of type dict_keys is not JSON serializable
Transition to shutdown state: Unhandled exception during run
Reactor garbage collection: (615.254495876, 0.0, 0.0)
Loaded MCU 'mcu' 87 commands (v0.9.1-13-g79877acb-20201029_092758-OctoPi / gcc: (GCC) 5.4.0 binutils: (GNU Binutils) 2.26.20160125)
MCU 'mcu' config: ADC_MAX=1023 BUS_PINS_spi=PB3,PB2,PB1 BUS_PINS_twi=PD0,PD1 CLOCK_FREQ=16000000 MCU=atmega2560 PWM_MAX=255 RECEIVE_WINDOW=192 RESERVE_PINS_serial=PE0,PE1 SERIAL_BAUD=250000 STATS_SUMSQ_BASE=256 STEP_DELAY=-1

@KevinOConnor
Copy link
Collaborator Author

Thanks. The last two reported errors should now be fixed.

-Kevin

@Piezoid
Copy link
Contributor

Piezoid commented Apr 6, 2021

I'm using this branch rebased on top of master on a regular basis. It sometimes requires small fixes when some updates on master break that. I will eventually batch them in a PR, but I don't want to bother @KevinOConnor each time.

If you encounter some difficulties rebasing this branch, feel free to check out my fork that may (or may not) be more current relative to master. But keep in mind that the combination with master changes and my modifications on top are only tested by me.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
The error checking is not complete in this change - the code should
handle the case where an input string is not valid utf8.

The code will continue to run on Python2 after this change, however
the execution time on Python2 is measurably slower after making this
change.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
@KevinOConnor KevinOConnor force-pushed the work-python3-20200612 branch 2 times, most recently from 034aadc to 4601f9d Compare October 2, 2021 01:18
@KevinOConnor
Copy link
Collaborator Author

I have changed this branch so that the code is now capable of running on either Python2 or Python3. I've also updated the automated build tests to run on both versions of Python. I intend to commit this branch in a few days.

This branch will not switch the installation to Python3. That will come at a later date and in a separate PR. In the interim, it is hoped that the updated build tests will catch any new code additions that result in a Python incompatibility.

Even after committing this branch, there is still likely to be areas of the code that are not compatible with Python3. If anyone finds a compatibility issue, please report it.

-Kevin

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Add wrappers for some common Python modules so that the code can run
on both Python2 and Python3.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Add a test case to verify that every optional module successfully
loads on both Python2 and Python3.  This is intended to catch syntax
and module imports that are not compatible between Python versions.

Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
Signed-off-by: Kevin O'Connor <kevin@koconnor.net>
@KevinOConnor KevinOConnor merged commit cc63fd5 into master Oct 6, 2021
@KevinOConnor KevinOConnor deleted the work-python3-20200612 branch October 6, 2021 17:23
@The-Monkey-King
Copy link

@wlhlm I don't see that as a bug as we should really close the door on Python 2 code. It is working as designed for Python 3 and is the proper coding going forward.

For outdated Klipper machines running Python 2, they have a choice: a) dead-end and maintain their firmware as-is without further support, or b) rebuild the host and client images with a fresh Python 3 build. It is possible to have Python 2 and 3 running concurrently, you just have to point to the Python 3 venv, instead of the other. There could be an upgrade installation but I think that's too much of a time sink and maintenance issue than just asking for a service & client rebuild.

@Sineos
Copy link
Collaborator

Sineos commented Jan 20, 2022

For outdated Klipper machines running Python 2, they have a choice: a) dead-end and maintain their firmware as-is without further support, or b) rebuild the host and client images with a fresh Python 3 build.

Klipper still is officially Python 2. This means even brand new installations will resort to a Python 2 venv. As long as this is the case, @wlhlm is correct and it should be considered a regression.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants