search

Klipper3d / klipper

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

Basic menu support (beta) #404

Closed mcmatrix closed 5 years ago

mcmatrix commented 6 years ago

Hi guys,

I wanted to inform you that I'm working on a simple menu system for klipper. I already have working mockup and now I'm trying to fit it into Klippers system.

The menu structure is fully configurable via config file. Menu is working on different screens, 2x16, 4x20 and on graphics displays. It's a text based menu system. You just have to specify correct cols and rows.

It has 3 basic menu elements:

All submenus have first item as "go back". System inserts it automatically.

You can define klipper system predefined variables in args attribute. In menu item name you can use variables list defined in args as standard print format options. For navigation only 3 buttons is needed: UP, DOWN, SELECT

When menu item (only input item) is in edit mode then system shows asterisk and UP & DOWN will change value accordingly. Currently only float value type is supported. Pressing SELECT again will exit from edit mode and will execute GCODE (value is given as print format first element).

I tried to make flexible way for adding dynamic data into name and gcode. There is attribute parameter: endstop.xmax:b["OPEN", "TRIG"] In Klipper there's collection of status info from all modules. First part endstop.xmax of attribute value is value key of that collection. Second part :b["OPEN", "TRIG"] of attribute value is additional way for mapping value to list or dict. : is delimiter b is boolean typecast for value. Other casting types are: f - float, i - int, b - bool, s - str ["OPEN", "TRIG"] is literal presentation of python list. List and Dict are allowed. Python will use safe method to turn it into real dataset.

You don't have to use mapping options all the time. In most cases it's not needed and just parameter with simple value key parameter: fan.speed will do.

When preparing name or gcode then you're able to use pyhton advanced string formatting possibilities. Value is avalilable as {0} and mapped option as {1}. If there's error condition in conversion or just wrong data type then raw name or gcode string is used instead of formatted one.

Changeable input is using value and can only be float type. But in name and gcode you can use mapped one if needed.

Menu manager config

[menu]
root: main1     # mandatory
rows: 4         # mandatory
cols: 20        # mandatory

Group menu item config

[menu main1]
type: group                 # mandatory
name: Main menu             # mandatory
enter_gcode:                # optional
leave_gcode:                # optional
items: menu1, menu2, menu3  # mandatory

Menu item with action

[menu menu1]
type: command                        # mandatory
name: Fan speed: {0:d}               # mandatory
parameter: fan.speed                 # optional

[menu menu2]
type: command          # mandatory
name: Pause octoprint  # mandatory
gcode: @//pause        # optional

[menu menu3]
type: command      # mandatory
name: Kill me      # mandatory
gcode: M112        # optional

[menu menu1]
type: command                               # mandatory
name: xmax endstop: {1:4s}                  # mandatory
parameter: endstop.xmax:b["OPEN", "TRIG"]   # optional

Menu item with input

[menu enable_fan]
type: input              # mandatory
name: Enable Fan: {0:d}  # mandatory
parameter: fan.speed     # mandatory
input_min: 0             # optional
input_max: 255           # optional
input_step: 1            # mandatory
gcode: M106 {0}          # optional

Some screenshots from mockup demo image image image

NB! This is still a Work in Progress. Concept, code, and other stuff might change!

mcmatrix commented 6 years ago

Interim report

Menu system is already working as Klipper module. Integration with display and buttons are still missing. Menu debugging is currently only possible with custom gcodes and terminal output. Code itself needs more work and debugging.

mcmatrix commented 6 years ago

@KevinOConnor, I'm having a problem with buttons.py module. I tried to checkout it and make but when I start klipper service it gives me an error:

self.mcu.send(self.ack_cmd.encode(new_count), cq=self.cmd_queue)
AttributeError: MCU instance has no attribute 'send'

Do you have any example code how to use this buttons module correctly?

KevinOConnor commented 6 years ago

On Mon, Jun 18, 2018 at 03:29:01PM -0700, Janar Sööt wrote:

Menu system is already working as Klipper module. Integration with display and buttons are still missing.

Thanks - it looks very interesting.

On Mon, Jun 18, 2018 at 10:39:08PM -0700, Janar Sööt wrote:

@KevinOConnor, I have problems with buttons.py module. I tried to checkout it and run but it gives me an error:

self.mcu.send(self.ack_cmd.encode(new_count), cq=self.cmd_queue)
AttributeError: MCU instance has no attribute 'send'

Do you have any example code how to use this buttons module correctly?

There was an error in one of the past merges of that branch. It should be fixed now.

I tested it with the following in my config:

[buttons] pins: ^!ar31, ^!ar33, ^!ar35, ^!ar41

With that, when I move the encoder clockwise, I get messages like the following in the log:

buttons=ar31 buttons=ar31,ar33 buttons=ar33 buttons=

with counter-clockwise I get:

buttons=ar33 buttons=ar31,ar33 buttons=ar31 buttons=

On pressing the encoder down I get:

buttons=ar35 buttons=

One challenge you will have with this demo code is that the handle_buttons_state() callback in buttons.py is invoked from a background thread. Some work will be needed to notify the main thread (which runs the display.py code).

-Kevin

mcmatrix commented 6 years ago

@KevinOConnor Hi, could you check my modifications for buttons module. https://github.com/mcmatrix/klipper/tree/work-buttons-20180620

I havent tested it yet, hopefully tomorrow i can do testings.

The idea is simple but not 100% sure that it will work. From other module you register your named button and pin in buttons module. Pin of course has to be in buttons monitoring list.

btns = config.get_printer().lookup_object('buttons')
btns.register_button('enc_a','ar31')
btns.register_button('enc_b','ar33')
btns.register_button('enc_c','ar35')

and in display update you can check registred buttons last press state btns.check_button('enc_c') What you think?

KevinOConnor commented 6 years ago

I suspect you may run into race conditions with that code (self.last_pressed isn't thread safe).

-Kevin

mcmatrix commented 6 years ago

I suspect you may run into race conditions with that code (self.last_pressed isn't thread safe).

Considering with thread safety will make things more complex. Queues are thread safe in Python. I have few ideas:

mcmatrix commented 6 years ago

I made short mockup with Queue.

import Queue

class error(Exception):
    pass

button_list = {}
button_list['enc_a'] = ('ar31', Queue.Queue(1))
button_list['enc_b'] = ('ar33', Queue.Queue(1))
button_list['enc_c'] = ('ar34', Queue.Queue(1))

pin_list = []
pin_list.append(('ar31', 1, 0))
pin_list.append(('ar33', 1, 0))
pin_list.append(('ar34', 1, 0))

# will be called from background thread
def handle_buttons_state(b):
    out_pins = []
    out_btns = []
    pressed_buttons = []
    pressed_pins = [pin for i, (pin, pull_up, invert) in enumerate(pin_list) if ((b>>i) & 1) ^ invert]

    print 'pins bits from mcu: %d' % b 

    for name, (pin, q) in button_list.items():        
        if pin in pressed_pins:
            pressed_buttons.append(name)
            try:
                q.put(name, False)
            except:
                pass

    out_pins.append(','.join(pressed_pins))
    out_btns.append(','.join(pressed_buttons))

    print "buttons_pins=%s" % ' '.join(out_pins)
    print "buttons_btns=%s" % ' '.join(out_btns)

# will be called from main thread
def check_button(name):
    press = None
    if name not in button_list:
        raise error("Button '%s' is not registered" % (name,))        
    try:
        press = button_list[name][1].get(False)
        button_list[name][1].task_done()
    except:
        pass
    return press

#---------------    
for b in range(1,4):
    handle_buttons_state(b)

#---------------
print 'check press: %s' % check_button('enc_a')
print 'check press: %s' % check_button('enc_a')

@KevinOConnor What you think, will this approach be thread safe?

mcmatrix commented 6 years ago

Interim report

buttons module - test version is ready for my testing menu module - test version is ready for my testing display module - both buttons and menu modules are integrated, needs my testing

Everything is still WIP!

mcmatrix commented 6 years ago

It's Alive!

It's in WIP statuses.

I'm having hard time with encoder stability. I first did encoder decoding in display module but the update rate was too low. So I moved encoder part to buttons module, inside pin state handler.

But still it's hard to get nice and stable encoder readout having that encoder functionality in buttons.py Can it be done in python level or it has to be done in mcu level?

I have 3 different dev branch:

Mostly these python files were modified by me.

My config is:

# Panel buttons
[buttons]
pins = ^!ar31, ^!ar33, ^!ar35, ^!ar41

# "RepRapDiscount 2004 Smart Controller" type displays
[display]
lcd_type: hd44780
rs_pin: ar16
e_pin: ar17
d4_pin: ar23
d5_pin: ar25
d6_pin: ar27
d7_pin: ar29
encoder_a_pin: ar31
encoder_b_pin: ar33
click_button_pin: ar35

# Menu manager
[menu]
root: main1
rows: 4
cols: 20

[menu main1]
type: group
name: Main menu
#enter_gcode:
#leave_gcode:
items: menu1, menu2, menu3

[menu menu1]
type: command
name: Resume octoprint
gcode: @//resume

[menu menu2]
type: command
name: Pause octoprint
gcode: @//pause

[menu menu3]
type: group
name: Group 1
#enter_gcode:
#leave_gcode:
items: menu4, menu5

[menu menu4]
type: command
name: Kill
gcode: M112

[menu menu5]
type: input
name: Move Z: {0:.2f}
parameter: toolhead.zpos
input_min: 0
input_max: 300
input_step: 0.01
gcode: G1 Z{0:.2f}

This is needed to run klipper with menu.

hg42 commented 6 years ago

from what I see here, the menu system always starts with a main menu, right? Could you allow to have a pure display page and use a button to go to the menu? Unfortunately, the menu configuration (especially according to dynamic parameters) seems to be based on menu lines, so it doesn't allow to have multiple parameters per line. I think this could be solved by using sections describing single parameters which could be inserted in the display via their names.

On a display page the encoder could jump from field to field. Pressing the encoder would activate editing the parameter via rotating the encoder, pressing again would leave editing. One of the fields could be a menu button, pressing the encoder on it would enter the menu system. There could be multiple display pages, switched by pressing on a button field (which could be the same as the menu button). Additionally display pages could be linked and scrolling through the parameters could automatically switch to the next or previous page.

Example config:

[menu page]
type: switch
parameter: menu.page
items: top,2nd

[menu X]
type: parameter
parameter: toolhead.xpos
input_min: 0
input_max: 300
input_step: 0.01
gcode: G1 X{0:.2f}

...

[menu top]
type: page
page:
  {page:s}   X{X:.2f}   Y{Y.2f}  Z{Z.2f}
  Bed{temp_bed:d} Hot{temp_hotend:d} ...

[menu 2nd]
type: page
...

Example usage:

>top   X:0   Y:0   Z:2.5
Bed:80° Hot:185° ...

[press]

>2nd ...

[press]

>top   X:0   Y:0   Z:2.5
Bed:80° Hot:185° ...

[right]

:top   X>0   Y:0   Z:2.5
Bed:80° Hot:185° ...

[press]

:top   X=0   Y:0   Z:2.5
Bed:80° Hot:185° ...

[right]

:top   X=1   Y:1   Z:2.5
Bed:80° Hot:185° ...

[press]

:top   X>1   Y:0   Z:2.5
Bed:80° Hot:185° ...

One of the pages could be the current type of menu. But I think, the menu could eventually replaced by using pages:

This would also allow to place several menu buttons on one line.

The selected field in the example is indicated by using a single character (mainly because I wanted to use simple text above). Instead, the selection could be indicated by inverting the field. However there needs to be another indicator for editing mode (underline?). The field could also be surrounded on both sides by spaces or [...] or >...< to indicate the three modes, but this would need another character for each field. Eventually the type of indicator could be chosen by a setting.

mcmatrix commented 6 years ago

from what I see here, the menu system always starts with a main menu, right?

Yes, it's like general marlin or repetier menu.

Thank you for sharing interesting menu concept. This horizontal layout for menuitems is nice and compact. Right now first priority is to get my menu working and get rid of WIP status. After that I'm ready to investigate what can be implemented from your idea.

mcmatrix commented 6 years ago

dev-release code is ready for testing

https://github.com/mcmatrix/klipper/tree/dev-release-20180622 dev-release contains all 3 merged work branches.

It's still beta firmware so use it ONLY FOR TESTING and not in production! Documentation is still missing.

Encoder is still problematic I tried to reduce encoder jitter with running average and new decoding algorithm but i'm not happy yet (anyway better than nothing). Dont try to rotate encoder very fast, it wont keep up.

Maybe someone has idea how to improve encoder decoding! Could be that we need to help python from mcu. In microcontroller level it should be able to detect encoder state changes very precisely?

Example config:

# Panel buttons
[buttons]
pins = ^!ar31, ^!ar33, ^!ar35, ^!ar41

# "RepRapDiscount 2004 Smart Controller" type displays
[display]
# "RepRapDiscount 128x64 Full Graphic Smart Controller" type displays
lcd_type: st7920
cs_pin: ar16
sclk_pin: ar23
sid_pin: ar17

#lcd_type: hd44780
#rs_pin: ar16
#e_pin: ar17
#d4_pin: ar23
#d5_pin: ar25
#d6_pin: ar27
#d7_pin: ar29
#encoder_a_pin: ar31
#encoder_b_pin: ar33
encoder_a_pin: ar33
encoder_b_pin: ar31
click_button_pin: ar35
encoder_resolution: 1

# Menu manager
[menu]
root: main1
rows: 4
#cols: 20
cols: 16

[menu main1]
type: group
name: Main menu
#enter_gcode:
#leave_gcode:
items: menu1, menu2, test1, menu3

[menu menu1]
type: command
name: Resume octoprint
#gcode: @//resume

[menu menu2]
type: command
name: Pause octoprint
#gcode: @//pause

[menu test1]
type: command
name: Printing: {1:>3s}
parameter: toolhead.is_printing:b['NO','YES']

[menu menu3]
type: group
name: Group 1
#enter_gcode:
#leave_gcode:
items: menu4, menu5, menu6

[menu menu4]
type: command
name: Kill
gcode: M112

[menu menu5]
type: input
name: Move Z: {0:.2f}
parameter: toolhead.zpos
input_min: 0
input_max: 300
input_step: 0.01
gcode: G1 Z{0:.2f}

[menu menu6]
type: input
name: Choose: {1:s}
parameter: 0:i('----', 'cola','pepsi','fanta','sprite')
input_min: 0
input_max: 4
input_step: 1
gcode: M117 My favorite: {1}
KevinOConnor commented 6 years ago

On Fri, Jun 22, 2018 at 02:38:29PM -0700, Janar Sööt wrote:

https://github.com/mcmatrix/klipper/tree/dev-release-20180622 dev-release contains all 3 merged work branches.

It's still beta firmware so use it ONLY FOR TESTING and not in production! Documentation is still missing.

Encoder is still problematic

Interesting. When I last tested this, I didn't see any indication of lost messages from the mcu even when I was spinning the rotary encoder very fast. I'll try to take a look at it - but it will likely take me a few days.

Thanks, -Kevin

mcmatrix commented 6 years ago

Did few changes in encoder handler, for me it looks more stable now, even without moving average (commented it out, probably not needed). Still fast spinning encoder states are not decoded correctly. dev-release-20180622 should be up to date.

Taz8du29 commented 6 years ago

First of all, this is awesome! This feature was definitely missing in klipper. Thanks for working hard on it.

Maybe someone has idea how to improve encoder decoding! Could be that we need to help python from mcu. In microcontroller level it should be able to detect encoder state changes very precisely?

I think that handling this at the mcu level would improve precision. Because of the latency of the serial connection, more than one trigger event can happen between two serial frames, so they aren't caught by the host software.

You can maybe append something like 3 characters at the end of the serial string for the different encoder infos:

nul : no encoder event
clk : encoder was rotated clockwise
ccw : encoder was rotated couter-clockwise
btn : encoder button was pressed

Another suggestion: Is putting the menus architecture in a separate, JSON-like file feasible?

Here are the advantages I see to this:

micheleamerica commented 6 years ago

I tried to install this WIP release to test the menus but I keep getting the following error when starting klippy: Config error Traceback (most recent call last): File "/home/pi/klipper/klippy/klippy.py", line 234, in _connect self._read_config() File "/home/pi/klipper/klippy/klippy.py", line 225, in _read_config option, section)) Error: Option 'encoder_a_pin' is not valid in section 'display'

Can a working example cfg file be posted to help in getting this to work?

mcmatrix commented 6 years ago

Latest dev-release should work and part of working config is posted above. Please update to the latest. Did you checkout dev-release fully? It looks like extras/display.py is not changed.

micheleamerica commented 6 years ago

I'll try a "from scratch" install of this dev-release (which in fact I did not do properly) and get back with some feedback.

mcmatrix commented 6 years ago

Install original klipper first. Set it up correctly, make sure that everything works and then check out my dev-release for testing. This way by using git checkout you can easily switch between different branches.

This should help with my dev-release checkout.

git remote add mcmatrix https://github.com/mcmatrix/klipper
git fetch mcmatrix
git checkout mcmatrix/dev-release-20180622

make clean
make

sudo service klipper stop
make flash FLASH_DEVICE=/dev/ttyACM0
sudo service klipper start
micheleamerica commented 6 years ago

Thanks for the heads-up. In the meantime I've a question concerning the pins required for the menus.

In the [buttons] section we see 4 pins being declared: pins = ^!ar31, ^!ar33, ^!ar35, ^!ar41

but then only 3 are in fact used for the encoder: encoder_a_pin: ar33 encoder_b_pin: ar31 click_button_pin: ar35

could you please explain this (apparent) inconsistence? Why is pin ar41 also declared?

In my current setup (Due + RADDS + MK4duo FW) I've got this as the pins being used for the encoder:

define BTN_EN1 50

#define BTN_EN2         52
#define BTN_ENC         48

Thanks in advance.

mcmatrix commented 6 years ago

In my case ar41 should be kill button but it’s not used right now. You can leave it out. In [buttons] section you have to list all pins (with config: pullups, inverted) you want to monitor. In [display] you just specify pins name (without config) you want to use for encoder.

micheleamerica commented 6 years ago

Thanks again for the clear explanation. I'm in the middle of a print… will try this after it finishes and get back to you.

mcmatrix commented 6 years ago

Those who have more navigation buttons can use these attributes too:

# enter, select, encoder click
click_button_pin: 

# up navigation, same as encoder up
up_button_pin: 

# down navigaton, same as encoder down
down_button_pin: 

# navigation back, return from active submenu, same as selecting  “..” in menu.
back_button_pin:

# needed for encoder
encoder_a_pin: 
encoder_b_pin:

NB! all these attributes can be used together.

micheleamerica commented 6 years ago

I've reinstalled Klipper and your branch and was able to start things up with your test configuration. On the other hand, as soon as I click the encoder button Klipper crashes and I have this in the log:

buttons: pins=ar48 ; buttons=click_button
buttons: pins= ; buttons=
Unhandled exception during run
Traceback (most recent call last):
  File "/home/pi/klipper/klippy/klippy.py", line 274, in run
    self.reactor.run()
  File "/home/pi/klipper/klippy/reactor.py", line 124, in run
    g_next.switch()
  File "/home/pi/klipper/klippy/reactor.py", line 151, in _dispatch_loop
    timeout = self._check_timers(eventtime)
  File "/home/pi/klipper/klippy/reactor.py", line 65, in _check_timers
    t.waketime = t.callback(eventtime)
  File "/home/pi/klipper/klippy/extras/display.py", line 535, in screen_update_event
    self.menu.begin(eventtime)
  File "/home/pi/klipper/klippy/extras/menu.py", line 187, in begin
    self.update_info(eventtime)
  File "/home/pi/klipper/klippy/extras/menu.py", line 221, in update_info
    info =  obj.get_heater().get_status(eventtime)
AttributeError: Heater instance has no attribute 'get_heater'

Any idea if this could come from my configuration or is it a real bug?

mcmatrix commented 6 years ago

Sorry, it seems like a bug. I'll fix it after I get home.

mcmatrix commented 6 years ago

@micheleamerica That bug should be fixed in dev-release-20180622.

micheleamerica commented 6 years ago

Great news. Thanks.

micheleamerica commented 6 years ago

Just updated and the issue is fixed. Thank you very much. BTW, you guys are doing a great job with Klipper… it is, until now, the best FW I ever installed in my 3d printers.

mcmatrix commented 6 years ago

@KevinOConnor I did some experiments with encoder decoding on mcu level. Please check mcu-encoder-test-20180625 test branch for this feature. Currently it's a more like hack, this bit twiddling makes my head hurt. Probably there's a nicer way of doing it.

It takes encoder a & b separate from button pins.

# Panel buttons
[buttons]
pins = ^!ar35, ^!ar41
encoder_pins = ^!ar33, ^!ar31

It decodes encoder input and makes encoder a&b pins behave like fake buttons. Decoder is nice state machine algorithm, look very reliable.

So encoder CW and CCW output is either pin A or pin B pressed.

In display you can use these:

[display]
lcd_type: hd44780
rs_pin: ar16
e_pin: ar17
d4_pin: ar23
d5_pin: ar25
d6_pin: ar27
d7_pin: ar29
encoder_up_pin: ar33
encoder_down_pin: ar31
click_button_pin: ar35

I had to reduce QUERY_TIME to .002 Right now fast spinning encoder looks okey. Will this faster query time affect printing performance?

KevinOConnor commented 6 years ago

On Mon, Jun 25, 2018 at 08:49:54PM +0000, Janar Sööt wrote:

@KevinOConnor I did some experiments with encoder decoding on mcu level. Please check mcu-encoder-test-20180625 test branch for this feature. Currently it's a more like hack, this bit twiddling makes my head hurt. Probably there's a nicer way of doing it. [...] I had to reduce QUERY_TIME to .002 Right now fast spinning encoder looks okey. Will this faster query time affect printing performance?

Okay, thanks. I got some time to retest this on the rotary encoder that came with my ST7920 display. I also now see the issue of spinning too fast causing lost events. (I'm not sure if the rotary encoder on my hd44780 display didn't have the issue, or if I just didn't test rotating that fast.) However, the only change that I think is necessary is to tune QUERY_TIME - I don't think moving the encoder logic to the mcu code is necessary.

I ran some benchmarks, and I don't see a significant difference in overhead between .005 and .002 so I think the faster time is okay.

I also cleaned up the buttons.c code a little (no more oid hacks). I've pushed the updated button code to the work-lcd-20180115.

I'm going to see if I can get the python code to forward events to the main thread so that the buttons code doesn't need to worry about threads.

-Kevin

KevinOConnor commented 6 years ago

FYI - I made some updates to the low-level button handling code. The demo code now forwards events to the main thread, and I updated the demo code to be able to run g-code on a button press.

The new code is on the work-lcd-20180115 branch. I tested it with the following config snippet:

[buttons encoder]
rotary_encoder_pins: ^!ar33, ^!ar31
clockwise_gcode:
    ECHO MSG=right
counterclockwise_gcode:
    ECHO MSG=left

[buttons encoder_press]
pin: ^!ar35
gcode:
    ECHO MSG=press
mcmatrix commented 6 years ago

Thank you. It's a nice upgrade. Now we can have for example kill button:

[buttons killme]
pin: ^!ar41
gcode:
    M112

I'll make changes in my work to integrate new buttons into menu system.

wizhippo commented 6 years ago

Not sure what direction you are going to take but add a custom gcode like MENU MSG=up/down/select/back may be a way to use the new button code. Then all we have to do is add the menu to the display if the menu exits. Haven't test but the idea is like https://github.com/KevinOConnor/klipper/compare/work-lcd-20180115...wizhippo:menu not sure is the menu cmd needs to call a method to update the display or just rely on it refreshing. EDIT: note I have made it so if the default group is selected the original display code will show. This allows to go back to that screen.

mcmatrix commented 6 years ago

It’s the simplest way but it depends how klipper is handling gcodes. For example if you are running M190 or M109. In marlin no other normal gcodes are executed until these are reached temp. I havent used klipper for real printing yet but probably it’s doing the same. When waiting gcode is running then you cannot use menu. How klipper will handle gcode burst when you spin encoder fast? At the moment I like more direct integration but lets see. I’ll test this gcode way of navigating menu.

KevinOConnor commented 6 years ago

I'd recommend doing the programming on the python level. Specifically, with the latest buttons code one can do the following in any module:

    buttons = printer.try_load_module(config, "buttons")
    buttons.register_button([config.get('my_pin')], self.my_callback)

to register a button and arrange for my_callback() to be invoked on each button press and release.

I fear going through g-code would add complexity and weird corner cases with command scheduling.

-Kevin

mcmatrix commented 6 years ago

@KevinOConnor Thank you. dev-release-20180622 is updated and encoder is running very well Now I can continue with menu testing.

For brave testers this should help you with my dev-release checkout. It's still beta firmware so use it ONLY FOR TESTING and not in production!

git remote add mcmatrix https://github.com/mcmatrix/klipper
git fetch mcmatrix
sudo service klipper stop
git checkout mcmatrix/dev-release-20180622

make clean
make

sudo service klipper stop
make flash FLASH_DEVICE=/dev/ttyACM0
sudo service klipper start

Part of my test printer config:

[buttons killme]
pin: ^!ar41
gcode:
    M112

# "RepRapDiscount 2004 Smart Controller" type displays
[display]
# "RepRapDiscount 128x64 Full Graphic Smart Controller" type displays
#lcd_type: st7920
#cs_pin: ar16
#sclk_pin: ar23
#sid_pin: ar17

lcd_type: hd44780
rs_pin: ar16
e_pin: ar17
d4_pin: ar23
d5_pin: ar25
d6_pin: ar27
d7_pin: ar29
encoder_pins: ^!ar33,^!ar31
click_pin: ^!ar35

# Menu manager
[menu]
root: main1
rows: 4
#cols: 20
cols: 16

[menu main1]
type: group
name: Main menu
#enter_gcode:
#leave_gcode:
items: menu1, menu2, test1, menu3

[menu menu1]
type: command
name: Resume octoprint
#gcode: @//resume

[menu menu2]
type: command
name: Pause octoprint
#gcode: @//pause

[menu test1]
type: command
name: Printing: {1:>3s}
parameter: toolhead.is_printing:b['NO','YES']

[menu menu3]
type: group
name: Group 1
#enter_gcode:
#leave_gcode:
items: menu4, menu5, menu6

[menu menu4]
type: command
name: Kill
gcode: M112

[menu menu5]
type: input
name: Move Z: {0:.2f}
parameter: toolhead.zpos
input_min: 0
input_max: 300
input_step: 0.01
gcode: G1 Z{0:.2f}

[menu menu6]
type: input
name: Choose: {1:s}
parameter: 0:i('----', 'cola','pepsi','fanta','sprite')
input_min: 0
input_max: 4
input_step: 1
gcode: M117 My favorite: {1}
mcmatrix commented 6 years ago

@KevinOConnor Could you make small change in buttons.py. Replace pin_params = ppins.lookup_pin('digital_in', pin) with pin_params = ppins.lookup_pin('digital_in', pin.strip())

KevinOConnor commented 6 years ago

On Wed, Jun 27, 2018 at 06:56:27AM +0000, Janar Sööt wrote:

@KevinOConnor Thank you. dev-release-20180622 is updated and encoder is running very well Now I can continue with menu testing.

For brave testers this should help you with my dev-release checkout. It's still beta firmware so use it ONLY FOR TESTING and not in production!

Great! I have some high level thoughts (dont't take them too seriously as I haven't had a chance to really look at your code in detail).

[buttons killme] pin: ^!ar41 gcode: M112 [...] [menu menu4] type: command name: Kill gcode: M112

I fear this wont work well because it would end up waiting for the current command to finish before executing the M112. I think a kill button (or menu item) would need to have some python code to make it work correctly.

I noticed in the code that you're using "self.gcode.run_script(script)". FYI, that isn't currently safe as it could lead to the g-code processor being reentrant. The self.gcode.process_batch() call is currently needed - as is used in buttons.py and virtual_sdcard.py. However, I think it's probably better to fix the gcode class instead of fixing this in the menu code

[...]

[menu main1] type: group name: Main menu

enter_gcode:

leave_gcode:

items: menu1, menu2, test1, menu3

One thing that I think would be useful is if the code defined a bunch of common menus by default. Then the config could override, extend, or replace the defaults if desired. That way casual users can get a basic menu without a lot of work. That said, no need to focus on that now.

On Wed, Jun 27, 2018 at 12:02:59AM -0700, Janar Sööt wrote:

@KevinOConnor Could you make small change in buttons.py. Replace pin_params = ppins.lookup_pin('digital_in', pin) with pin_params = ppins.lookup_pin('digital_in', pin.strip())

Sure. In fact, ppins.lookup_pin() should be doing the strip itself. I'll take a look at it.

Finally, you may have noticed that I moved the display.py code to its own sub-directory in extras/. I hope this doesn't conflict too badly with the work you've been doing. I wanted to get the code separated so that it would be easier to merge future changes. Along those lines, I wonder if menu.py could be moved to klippy/extras/display/menu.py, have it instantiated by display.py, and have it use config.get_prefix_sections('menu ') to find all the menu blocks in the config (instead of using load_config() ).

-Kevin

mcmatrix commented 6 years ago

I fear this wont work well because it would end up waiting for the current command to finish before executing the M112. I think a kill button (or menu item) would need to have some python code to make it work correctly.

Is it possible to make it high priority gcode command? So when gcode parser sees it then it will act immediately and it doesnt matter what else is going on. It's also better for octoprint and repetier-server. They have "emergency stop" button and it's calling M112 gcode.

The self.gcode.process_batch() call is currently needed - as is used in buttons.py and virtual_sdcard.py. However, I think it's probably better to fix the gcode class instead of fixing this in the menu code I'll try to take a look at that.

OK I'll check how it's done in buttons and virtual_sdcard and do the same.

One thing that I think would be useful is if the code defined a bunch of common menus by default. Then the config could override, extend, or replace the defaults if desired. That way casual users can get a basic menu without a lot of work. That said, no need to focus on that now.

Actually it's a good idea to have standard common menu. But what should be in that standard default menu for klipper? This is probably topic for the next discussion-issue.

I hope this doesn't conflict too badly with the work you've been doing.

No problem, i can handle it.

I wonder if menu.py could be moved to klippy/extras/display/menu.py, have it instantiated by display.py, and have it use config.get_prefix_sections('menu ') to find all the menu blocks in the config (instead of using load_config() ).

OK did i understand it correctly. You're suggesting to get rid of [menu] section

[menu]
root: main1
rows: 4
cols: 20

move these attributes under [display] section and have it to initiate menu manager and load all the menu items? I think it's doable. Btw how should I initiate menu class itself (like buttons module)? self.menu = self.printer.try_load_module(config, "menu") When I'm looking into current display.py then it seems that I should just import menu and initiate it as normal class?

KevinOConnor commented 6 years ago

On Thu, Jun 28, 2018 at 12:02:51AM -0700, Janar Sööt wrote:

I fear this wont work well because it would end up waiting for the current command to finish before executing the M112. I think a kill button (or menu item) would need to have some python code to make it work correctly.

Is it possible to make it high priority gcode command? So when gcode parser sees it then it will act immediately and it doesnt matter what else is going on. It's also better for octoprint and repetier-server. They have "emergency stop" button and it's calling M112 gcode.

Klipper does do that when reading input, but it does not have that logic in runscript() or process_batch() though. It's complex to scan commands out of order, so I don't think it makes sense to add that complexity to runscript(). Easier (and safer) to have code that directly invokes the shutdown logic ( printer.invoke_shutdown() ).

OK did i understand it correctly. You're suggesting to get rid of [menu] section

[menu]
root: main1
rows: 4
cols: 20

move these attributes under [display] section and have it to initiate menu manager and load all the menu items? I think it's doable. Btw how should I initiate menu class itself (like buttons module)? self.menu = self.printer.try_load_module(config, "menu")

I was thinking something more like:

import menu ... self.menu = menu.Menu(config)

BTW, the display should know the rows and columns (hd44780 is 20x4, all others are 16x4), so unless I'm missing something it shouldn't be necessary for the user to specify that in the config.

-Kevin

mcmatrix commented 6 years ago 
BTW, the display should know the rows and columns (hd44780 is 20x4,
all others are 16x4), so unless I'm missing something it shouldn't be
necessary for the user to specify that in the config.

Yes and no. The ready made RepRapDiscount Smart Controller (hd44780) has 20x4 and RepRapDiscount Full Graphic Smart Controller (builtin character generator) has 16x4. But hd44780 can also have 16x4, 16x2, 8x2 etc variations. Any other full graphic lcd using custom fonts can be different than 16x4. We can make cols,rows to use default values by LCD_chips. So that in common cases user doesn't have to specify these. But lets leave option to specify different rows and cols if needed. There're lot of DIY doers.

mcmatrix commented 6 years ago

My dev-release-20180622 is updated to the latest. Menu is now under display modules. Encoder decoding is inside buttons.py RotaryEncoder class Display is using that class from buttons. Added kill_pin for emergency stop. Menu run_scripts is using self.gcode.process_batch()

In config there's no more [menu] section, only menuitems remain. Display config with menu looks like

# "RepRapDiscount 2004 Smart Controller" type displays
[display]
# "RepRapDiscount 128x64 Full Graphic Smart Controller" type displays
#lcd_type: st7920
#cs_pin: ar16
#sclk_pin: ar23
#sid_pin: ar17
lcd_type: hd44780
rs_pin: ar16
e_pin: ar17
d4_pin: ar23
d5_pin: ar25
d6_pin: ar27
d7_pin: ar29
root: main1
encoder_pins: ^!ar33,^!ar31
click_pin: ^!ar35
kill_pin: ^!ar41
mcmatrix commented 6 years ago

Is it possible to use RepRapDiscount controller buzzer for having beeper? It doesnt have to be that fancy like in marlin. Just to do short beeps is enought.

AxMod3DPrint commented 6 years ago

@mcmatrix - not sure code wise, but it is just a pin, so I don't think implementing it should be too difficult. I had some issues with some bleed back on the pin, so it's possible to set the pin high and low without much issue via set_pin

mcmatrix commented 6 years ago

@AxMod3DPrint - I'll try that. It depends is it active or passive buzzer. With active buzzer it's easy just pin on/off. If it's passive then it's more complex you have to generate frequency for it.

AxMod3DPrint commented 6 years ago

It's just active on the Reprap Full Graphic and Reprap Smart, so it really is just an on/off

mcmatrix commented 6 years ago

@hg42 New menuitem type row is available. By using this you can make compact menu layouts having multiple items in one row. It accepts command and input type items and it show items in one row. It uses blinking as selecting indicator (underline is not possible on text screen).

Example menu config

# "RepRapDiscount 2004 Smart Controller" type displays
[display]
# "RepRapDiscount 128x64 Full Graphic Smart Controller" type displays
#lcd_type: st7920
#cs_pin: ar16
#sclk_pin: ar23
#sid_pin: ar17

lcd_type: hd44780
rs_pin: ar16
e_pin: ar17
d4_pin: ar23
d5_pin: ar25
d6_pin: ar27
d7_pin: ar29
root: main1
encoder_pins: ^!ar33,^!ar31
click_pin: ^!ar35
kill_pin: ^!ar41

[menu main1]
type: group
name: Main menu
#enter_gcode:
#leave_gcode:
items: menu1, menu2, test1, menu20, menu3

[menu menu1]
type: command
name: Resume octoprint
#gcode: @//resume

[menu menu2]
type: command
name: Pause octoprint
#gcode: @//pause

[menu test1]
type: command
name: Printing: {1:>3s}
parameter: toolhead.is_printing:b['NO','YES']

[menu menu3]
type: group
name: Group 1
#enter_gcode:
#leave_gcode:
items: menu4, menu5, menu6

[menu menu4]
type: command
name: Kill
gcode: M112

[menu menu5]
type: input
name: Move Z: {0:.2f}
parameter: toolhead.zpos
input_min: 0
input_max: 300
input_step: 0.01
gcode: G1 Z{0:.2f}

[menu menu6]
type: input
name: Choose: {1:s}
parameter: 0:i('----', 'cola','pepsi','fanta','sprite')
input_min: 0
input_max: 4
input_step: 1
gcode: M117 My favorite: {1}

[menu menu20]
type: row
items: menu21,menu22,menu23

[menu menu21]
type: input
name: X:{0:.2f}
parameter: toolhead.xpos
input_min: 0
input_max: 300
input_step: 0.01
gcode: G1 X{0:.2f}

[menu menu22]
type: input
name: Y:{0:.2f}
parameter: toolhead.ypos
input_min: 0
input_max: 300
input_step: 0.01
gcode: G1 Y{0:.2f}

[menu menu23]
type: input
name: Z:{0:.2f}
parameter: toolhead.zpos
input_min: 0
input_max: 300
input_step: 0.1
gcode: G1 Z{0:.2f}

It will show as one row: >X:0.00Y:0.00Z:0.00 You can select them as normal menuitems.

It can be still buggy!

This is available for testing in dev-release-20180622

mcmatrix commented 6 years ago

I propose initial structure for default menu

- Pause Octoprint
- Resume Octoprint
- Abort Octoprint
- SD Card print
    - Start/resume
    - Pause
    - list of files
- Control
    - Home Z
    - Home X/Y
    - Move 10mm
        Move X: 000.0
        Move Y: 000.0
        Move Z: 000.0
        Move E: 000.0
    - Move 1mm
        Move X: 000.0
        Move Y: 000.0
        Move Z: 000.0   
        Move E: 000.0       
    - Move 0.1mm
        Move X: 000.0
        Move Y: 000.0
        Move Z: 000.0
        Move E: 000.0
    - Fan: ON/OFF
    - Motors off
- Temperature
    - Hotend: 23°
    - Hotbed: 24°   
    - Preheat PLA
        - Preheat all
        - Preheat hotend
        - Preheat hotbed
    - Preheat ABS
        - Preheat all
        - Preheat hotend
        - Preheat hotbed
    - Cooldown
        - Cooldown all
        - Cooldown hotend
        - Cooldown hotbed
- Filament
    - Preheat hotend
    - Unload Filament
    - Load Filament
    - Feed Filament
- Prepare
    - Delta calibrate
    - Bed Tilt
    - Z Tilt

This will be the menu structure in example-menu.cfg It's not meant to be final. You can always customize that structure to meet your needs.

AxMod3DPrint commented 6 years ago

Looks pretty comprehensive to me.

Once thing that's crossed my mind, but would probably a whole lot of work, would be pulling the file list from the Virtual SD folder and being able to start and stop print via that. That would probably bring even more accessibility to klipper by having those that do not like to run teathered via octoprint or whatever front end. Just means you could CURL or SCP the file directly to the virtual SD folder on the host and walk over to the printer and start the print.

On 30 June 2018 at 13:34, Janar Sööt notifications@github.com wrote:

I propose initial structure for default menu

  • Pause printing
  • Resume printing
  • Control
    • Home Z
    • Home X/Y
    • Move 10mm Move X: 000.0 Move Y: 000.0 Move Z: 000.0 Move E: 000.0
    • Move 1mm Move X: 000.0 Move Y: 000.0 Move Z: 000.0
      Move E: 000.0
    • Move 0.1mm Move X: 000.0 Move Y: 000.0 Move Z: 000.0 Move E: 000.0
    • Fan: ON/OFF
    • Motors off
  • Temperature
    • Hotend: 23°
    • Hotbed: 24°
    • Preheat PLA
      • Preheat all
      • Preheat hotend
      • Preheat hotbed
    • Preheat ABS
      • Preheat all
      • Preheat hotend
      • Preheat hotbed
    • Cooldown
      • Cooldown all
      • Cooldown hotend
      • Cooldown hotbed
  • Filament
    • Preheat hotend
    • Unload Filament
    • Load Filament
    • Feed Filament
  • Prepare
    • Delta calibrate
    • Bed Tilt
    • Z Tilt

This will be the menu structure in example-menu.cfg It's not meant to be final. You can always customize that structure to meet your needs.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/KevinOConnor/klipper/issues/404#issuecomment-401538425, or mute the thread https://github.com/notifications/unsubscribe-auth/AQULnn8EtabxIdJRcyUdWx5wCnKqA2Gqks5uB3A8gaJpZM4Uq-yU .

mcmatrix commented 6 years ago

@AxMod3DPrint You're right, it's probably lot of work to integrate with menu but I'll check that virtual sd stuff. EDITED: Looks quite doable! It's a simple list of files. Folders are not supported atm.

Btw RepRapDiscount has active buzzer so this will work.

[output_pin buzzer]
pin: ar37

[gcode_macro BEEP]
gcode:
    SET_PIN PIN=buzzer VALUE=1
    G4 P200
    SET_PIN PIN=buzzer VALUE=0

This G4 P200 - i added 200ms pause but it's ok without it too Now BEEP gcode can be easily inserted into print start & end scripts.