Klipper3d / klipper

Klipper is a 3d-printer firmware
GNU General Public License v3.0
9.51k stars 5.32k forks source link

A proposal: major code update. #2683

Closed mfp20 closed 4 years ago

mfp20 commented 4 years ago

First thing first: please close preliminary investigations and proposal, as this is the conclusion of our previous writings.

Then the serious stuff: I've pushed my rework of klippy in my repo for evaluation purposes. It's far from being ready to work/distribute. I've just seen a log that looks promising on my test-bed (1 desktop cpu, 3 simulavr mcus) and posted for evaluation. To give a glance, the log is attached.

What's new? In short: I tried to increase configuration flexibility without writing Yet Another Generic Robotic Toolkit (nobody feels the need of).

In detail I've been working on two items instead:

  1. Software development

standards

  1. Features (to be completed)

Moreover, combining the "compose any kind of printer" and "can suspend at any time", takes us fast to "easy embedding of metal&electronics in extruded parts".

Most of the items above have to be completed but the code is good enough to show where I'm going. Performance wise I get weird results looking at "ps" output. Looks like memory footprint didn't increase; I was expecting a huge increase instead. Probably I've butchered something important :)

What about including this code in his own branch (ex: klippy-next) so that others can participate? Does it look good enough to you? Feel free to be rude. I'm not a python pro but ... better something than nothing.

cheers

mfp20 commented 4 years ago

From plugins/example_module.py

# Init procedure:
# 1. user runs Klippy
# 2. klippy, reads config and starts composer
# 3. composer, assembles parts, create the tree root and basic printer's facilities, then loads all the modules. All plugins are added as "part". All nodes have a module.
# 4. composer, calls load_tree_node(hal, node) for each and all modules having such method. All nodes are in place.
# 5. hal, calls load_node_object() for each and all nodes having such method in their module. All nodes have a bare minimum object.
# 6. hal, calls build() for each and all composites having such method in their module. Printer's shallow children first, toolhead after.
# 7. build(), calls configure() for each and all leaves having such method in their module. All simple parts are ready. 
# 8. hal, calls init() for each and all composites having such method in their module. All composites are ready.
# 9. hal, calls register() for each and all instantiated in-tree objects having such method. All events and commands are registered to their own managers.
# 10. Init is over. Klippy goes back in control and will start accepting jobs... 
mfp20 commented 4 years ago

Now each temperature controller supports multiple sensors/heaters/coolers and could possibly be functional. But worth nothing as extruders class is not ready yet. Basically if multiple sensors are in the same tcontrol branch, they will be averaged; if multiple heaters are in the same branch, they all share the same governor (ie: pwm output), and it is useful for some printers having multiple heaters on the same (big) heat bed. Coolers need a bit more love; now share the same pwm value as well, but the value is "inverted" before apply the new pwm value to the coolers. Need more time to think and test the whole "cooling" thing. There's also a "dew point" calculator, and a barometer stub I placed there before searching for dew point formulas... I thought "dew point" was function of temp+humidity+pressure, instead at sea level is temp+humidity only. In any case, the best option is to use a tcontrol per each sensor+actuator pair, as the tcontrol can be placed anywhere in the printer tree...

I have a some nasty bug "ADC out of range" but I can't understand what's happening. Looks like the simulavr (or the firmware built for it) is broken; it complains about an adc register not built in.

I keep going...

mfp20 commented 4 years ago
mfp20 commented 4 years ago

Added '-c' start option...

    opts.add_option("-c", action="store_true", dest="printerconsole",
                    help="spawn a printer command interactive console, implies '-l klippy.log'")

It's a simple console made with cmd.Cmd module.

    def help_README(self):
        help_txt = """
            This is the main Klippy console. Exiting this console closes gracefully the whole Klippy app.
            The console features "tab completion": tapping TAB will complete the current word, double TAB will give you hints.
            Some commands make you enter sub-consoles, each with its own set of commands. Example: "toolhead" and "mcu".
            There is also a subset of OS shell commands (ex: ls, cd, cat), just prepend the OS shell command with "!" or "shell",
            Example: "! ls"
        """
        print 'README'
        print help_txt

... and removed the old console.py. Currently the mcu console is the only working. The commander's consoles are just stubs. As soon as the "commander" will be working, all commands available at /tmp/printer (or octoprint) will be available in the console as well (if the console has been requested on startup). It's great for testing... the previous one was a bit ... bare ...

A sample session:

user@user-klipper-devel:~/next$ /home/user/klippy-env/bin/python klippy/klippy.py printer.cfg -c -v

 * Klippy command console. Type 'help' or '?' to list currently available commands.

Klippy > help

Documented commands (type help <topic>):
========================================
continue  exit  help  mcu  reload  restart  shell  toolhead

Miscellaneous help topics:
==========================
README

Klippy > help help
List available commands with "help" or detailed help with "help cmd".
Klippy > help exit
Exit this (sub-)console.
You can also use the Ctrl-D shortcut.
Klippy > help mcu
MCU sub-console. To exit just type "exit" and you will be back to main Klippy console.
Klippy > mcu arm1

 * MCU 'arm1' command console. Type 'help' or '?' to list currently available commands.

Klippy:mcu arm1 > connect
Loaded 80 commands (v0.8.0-409-g0342c500-20200403_213401-user-klipper-devel / gcc: (GCC) 5.4.0 binutils: (GNU Binutils) 2.26.20160125)
MCU config: RECEIVE_WINDOW=192 BUS_PINS_spi=PB6,PB5,PB7 SERIAL_BAUD=250000 RESERVE_PINS_serial=PD0,PD1 STEP_DELAY=-1 ADC_MAX=1023 PWM_MAX=255 CLOCK_FREQ=20000000 BUS_PINS_twi=PC0,PC1 MCU=atmega644p STATS_SUMSQ_BASE=256
Klippy:mcu arm1 > get_uptime
Klippy:mcu arm1 > 015.394: uptime high=0 clock=1752832989

Klippy:mcu arm1 > get_config
Klippy:mcu arm1 > 018.763: config is_config=0 crc=0 move_count=0 is_shutdown=0

Klippy:mcu arm1 > stats
bytes_write=602 bytes_read=3269 bytes_retransmit=9 bytes_invalid=0 send_seq=81 receive_seq=81 retransmit_seq=11 srtt=0.014 rttvar=0.006 rto=0.036 ready_bytes=0 stalled_bytes=0 freq=2405689
Klippy:mcu arm1 > help

Intro
=====
This is a mcu debugging console for the Klipper micro-controller.
The "connect" command is automatically issued once at first given command
to collect all handles and information. Disconnect is automatic on exit.

In addition to console commands, you can issue mcu commands.
Example: "get_uptime".
All commands also support evaluation by enclosing an expression in { }.
Example: "reset_step_clock oid=4 clock={clock + freq}"
In addition to user defined variables (via the SET command) 
the following builtin variables may be used in expressions:
    clock : The current mcu clock time (as estimated by the host).
    freq  : The mcu clock frequency.

Documented commands (type help <topic>):
========================================
connect  delay  disconnect  exit  flood  help  set  stats  suppress

Available mcu commands:
=======================
allocate_oids count=%c
buttons_ack oid=%c count=%c
buttons_add oid=%c pos=%c pin=%u pull_up=%c
buttons_query oid=%c clock=%u rest_ticks=%u retransmit_count=%c invert=%c
clear_shutdown
config_analog_in oid=%c pin=%u
config_buttons oid=%c button_count=%c
config_digital_out oid=%c pin=%u value=%c default_value=%c max_duration=%u
config_endstop oid=%c pin=%c pull_up=%c stepper_count=%c
config_hd44780 oid=%c rs_pin=%u e_pin=%u d4_pin=%u d5_pin=%u d6_pin=%u d7_pin=%u delay_ticks=%u
config_i2c oid=%c i2c_bus=%u rate=%u address=%u
config_neopixel oid=%c pin=%u
config_pwm_out oid=%c pin=%u cycle_ticks=%u value=%hu default_value=%hu max_duration=%u
config_soft_pwm_out oid=%c pin=%u cycle_ticks=%u value=%c default_value=%c max_duration=%u
config_spi oid=%c pin=%u
config_spi_shutdown oid=%c spi_oid=%c shutdown_msg=%*s
config_spi_without_cs oid=%c
config_st7920 oid=%c cs_pin=%u sclk_pin=%u sid_pin=%u sync_delay_ticks=%u cmd_delay_ticks=%u
config_stepper oid=%c step_pin=%c dir_pin=%c min_stop_interval=%u invert_step=%c
config_thermocouple oid=%c spi_oid=%c thermocouple_type=%c
config_tmcuart oid=%c rx_pin=%u pull_up=%c tx_pin=%u bit_time=%u
debug_nop data=%*s
debug_ping data=%*s
debug_read order=%c addr=%u
debug_write order=%c addr=%u val=%u
emergency_stop
end_group
endstop_home oid=%c clock=%u sample_ticks=%u sample_count=%c rest_ticks=%u pin_value=%c
endstop_query_state oid=%c
endstop_set_stepper oid=%c pos=%c stepper_oid=%c
finalize_config crc=%u
get_clock
get_config
get_uptime
hd44780_send_cmds oid=%c cmds=%*s
hd44780_send_data oid=%c data=%*s
i2c_modify_bits oid=%c reg=%*s clear_set_bits=%*s
i2c_read oid=%c reg=%*s read_len=%u
i2c_write oid=%c data=%*s
identify offset=%u count=%c
neopixel_send oid=%c data=%*s
query_analog_in oid=%c clock=%u sample_ticks=%u sample_count=%c rest_ticks=%u min_value=%hu max_value=%hu range_check_count=%c
query_thermocouple oid=%c clock=%u rest_ticks=%u min_value=%u max_value=%u
queue_step oid=%c interval=%u count=%hu add=%hi
reset
reset_step_clock oid=%c clock=%u
schedule_digital_out oid=%c clock=%u value=%c
schedule_pwm_out oid=%c clock=%u value=%hu
schedule_soft_pwm_out oid=%c clock=%u on_ticks=%u
set_digital_out pin=%u value=%c
set_next_step_dir oid=%c dir=%c
set_pwm_out pin=%u cycle_ticks=%u value=%hu
spi_send oid=%c data=%*s
spi_set_bus oid=%c spi_bus=%u mode=%u rate=%u
spi_set_software_bus oid=%c miso_pin=%u mosi_pin=%u sclk_pin=%u mode=%u rate=%u
spi_transfer oid=%c data=%*s
st7920_send_cmds oid=%c cmds=%*s
st7920_send_data oid=%c data=%*s
start_group clock=%u
stepper_get_position oid=%c
tmcuart_send oid=%c write=%*s read=%c
update_digital_out oid=%c value=%c

Available local variables:
==========================
  clock: 1806153585
  freq: 20000000.0
Klippy:mcu arm1 > exit
Klippy > ! ls
avrsim1.vcd  avrsim3.vcd      klippy      printer.cfg  simple.printer.cfg  start.console.sh  test        TODO.logic
avrsim2.vcd  extras_to_be_ported  klippy.log  README.md    start.avrsim.sh     start.klippy.sh   TODO.devel
Klippy > exit

Close Klippy and return to OS shell? (yes/no):yes
exit
user@user-klipper-devel:~/next$
KevinOConnor commented 4 years ago

FWIW, my high-level feedback is that this isn't something I plan to look at in the short term. My suggestion for those looking to start on Klipper development is to work on a new feature that adds additional user visible functionality - in my experience, "code cleanups" are too prone to "bike shedding" and aren't a good place to start in a project.

Also, FWIW, these changes modify the config file and it would be a large ask to have all Klipper users rework their config file without a tangible user facing gain. So, it isn't something I would pursue.

-Kevin

mfp20 commented 4 years ago

@KevinOConnor I agree. I didn't ask to trash your bike; I just want to park my moped nearby. I wrote it in the previous thread: "nothing to merge anytime soon". Just create a branch "klippy-next" using your actual code base, and forget it. So I can rebase my code on top of that, and ping you using a pull request only when I'll have my code up&running on my printer. The idea is to let you keep going on the 0.8 branch without bothering you further; so one day you'll have a 1.0! At that point, my code could be a good starting point for a 2.0. COULD. If it'll be, you'll have continuous code history from 1.0 to 2.0. If not good for you, you will not accept my pull request, and your branch klippy-next will be pristine for you to start the 2.0 development.

TL;TR (just for talking sake)

About "modify the config file": no user is forced to update their software - I'm not Bill Gates and I don't have a "Microsoft Genuine Advantage" program - I'm not suggesting to move development to the other branch. They (and you all!!!) can keep going with their 1.0 branch. One of the most important computing mantras is: "If it works, DON'T TOUCH IT, STUPID!". And I'm not that stupid guy. There's nothing to switch to, now; but a major code update will be needed soon, IMHO.

About "user facing gain": the current "user API" is crap. It takes a while to learn about "firmware restarts" and "restarts"; the right sequence of commands to give on octoprint, depending on the ongoing procedure (ex: calibrating heaters or steppers, or print): every procedure have some gotchas to learn. And the printer goes offline at every hicup. Basically the extra user comfort given by not having to re-compile/re-update C firmware, is gone as soon as we start to use the printer. I've already rewritten klippy.py in order to have a clean "reload" (ie: reload config and restart) and "restart" (ie: restart using the same config), without loosing connection to octoprint. This is a small change, but nice gain for a user. And there are other small "user gains" as well: a console to manage the printer without (the utterly uncomfortable) octoprint or do it concurrently to octoprint (if you know what you are doing): currently you need to echo commands to the tty device and decode its output. And then the chance to use multiple endstops, multiple extruders, in many different setups: currently min-max endstops as well as some multi-extruder setups aren't supported, or they are with a lot of pain.

And from the developer perspective is even worse: it takes pretty long time before being able to "add feature". I added docstrings to most of the classes and functions, instead; centralized messaging and error handling. Have a look at some C-project from the 80s and it's rewrite in C++ in the 90s-00s. Then look at current klippy, and some other mature python project. You'll get my point. Don't get me wrong: I'm not a good developer, I'm sure there are better solutions out there. And again: I'm not saying that the actual software is crap, it's not; I'm here because I simply love it. Ideally, once you got your 1.0, that one would be the perfect solution for common printers (ie: 90% of the printers out there?); the 2.0 draft, a fair option for anything else.

Further. On my local repo I switched to python3 (and to argsparse from optsparse). A major update that doesn't look like a "user facing gain": it will be, the (not so far) day python3 will be the default&only on all distros. But Python 3 have a fixed "GIL-time" of 15ms; this could make your reactor blow out. And all the popular SBCs are quadcore (ex: Rpi3 and Rpi4) or more (ex: I'm using an odroid-xu4, 8 cores): why to keep using co-routines (greenlet)? That was the right choice in 2016; now it's not. Now a meditated mix of multiprocessing, multithreading and asyncio can exploit this new SMP hardware; and add the chance to get rid of USB (ie: use I2C/SPI for host2mcu communication, cable length permitting). So I rewrote the reactor using concurrent multiprocessing instead of threads/greenlets. BTW: using multiprocessing helps one of my other design rule (ie: use pickle). Currently the multiprocessing scheduler (mp-reactor) can exec a timer callback within 200us from the trigger: it looks fast enough to replace the old reactor, and has concurrent.futures.ProcessThreadPool for async tasks. Moreover there's the chance to pin the processes to cores (ex: my odroid has 4 high power cores and 4 low power ones; the kernel does a good job in ondemand switching cores, but some pinning could be needed once octoprint is competing for core-time). Cherry on yop: the mp-reactor falls-back to your reactor in case the cpu cores are less than 4, or the user requested at start to run in single-process mode. I'm currently finishing this reactor, then I'll make the old and the new having the same fingerprint, then add my previous stuff, then I'll commit to github again.

I'm keeping as much as possible of your code; not just because I'm bound to the chelper part, but to ease the 1.0->2.0 transition for actual developers/users too. Time management and toolhead movement is ... pristine. I didn't touch it. For me the core stuff is a black box that just works good as is. Just a bit blobby, so I made a skeleton for it to get a well known shape.

D4SK commented 4 years ago

I find this very promising, opening up klipper for CNCs, and all possible multi-extruder setups. I think support for single core cpus isn't needed though, octoprint runs terrible on rpi2 and in the future everything will be multicore anyways. I'd rather focus on not introducing additional complexity into the codebase.

I think the new config is much more complex though and categorizing some modules as tool or sensor might be an unnecessary layer for the user. This is not a robotics framework after all.

[tool hblock3]
type: tcontrol
sensor: hblock3
heater: hblock3
control: pid
...

should be: [temperature_control hblock3] thermometer: hblock3 heater: hblock3 control: pid ...

and [tool e1] type: extruder stepper: e1 tool: hblock1 nozzle: n1 ...

should be [extruder e1] stepper: e1 temperature_control: hblock1 nozzle: n1 ...

mfp20 commented 4 years ago

@D4SK thanks for the feedback. You are the first&only one :)

Currently in my repo there is the forth complete re-write: nothing is definitive. Unfortunately I had to learn both python and klippy. And I like the semi-empirical approach so... studied while writing. The results are idiosyncrasies, convolution, unnecessary complexity, and so on; so, after a full rewrite, there's always urgent need of another rewrite to clear the crap. And again. And again ... As soon as I'm satisfied with the current result, I'll go ahead to reach the "working status".

That said:

jneilliii commented 4 years ago

Just curious, is this the first branch/fork that is Python 3 compatible @mfp20?

mfp20 commented 4 years ago

@jneilliii AFAIK, it's not. Kevin clearly stated he doesn't want to take risk of loosing features or break the current code. Nor to spend time on major rewrites without advantages for the current users. This is what I got when he wrote (2 times) to avoid "bike shedding". So, whatever I'm doing is not a branch, yet; it's a standalone effort, not even a project itself. I hope once my "stuff" is working, he'll make a branch for me and accept my code under his shelter.

About Python3: no idea if someone else forked to make it python3 based. For me using python3 is just a side effect of making it at this time; it's just mandated by the times we're living in. Once there are enough reasons to allocate time for a full rewrite, to use python2 is silly; because python2 has been EOL'ed from all major distros.

My advice for the moment is to pretend my code doesn't exist: users can't use it as it doedsn't work, developers better don't waste time as any code could change deeply any time.

When it will be working for me, I'll freeze the code and issue a "call to arms" to help porting the missing features (that I don't need of, but are needed in order to support the current users).

KevinOConnor commented 4 years ago

Any further updates on this? I'm not sure we need to track this as an open item in Klipper.

-Kevin

mfp20 commented 4 years ago

Any further updates on this? I'm not sure we need to track this as an open item in Klipper.

Not any new code. I've been busy in the past two months and just went back yesterday to work on this project.

The python3 daemon is mostly done; further work is needed in order to make it talk to the C firmware, then debug.

I'm reworking the C firmware now:

About keeping it open here: it's up to you. There's no real need of this thread to stay open. Can do a new one when the code is worth a glance.

Thanks!