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

Basic menu support (beta) #404

Closed
mcmatrix opened this issue Jun 17, 2018 · 135 comments
Closed

Basic menu support (beta) #404

mcmatrix opened this issue Jun 17, 2018 · 135 comments

Comments

@mcmatrix
Copy link
Contributor

mcmatrix commented Jun 17, 2018

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:

  • Group menu item (folder)
    SELECT will enter into submenu
  • Menu item with action
    SELECT will execute GCODE (if specified)
  • Menu item with input
    SELECT will go into edit mode

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
Copy link
Contributor Author

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
Copy link
Contributor Author

mcmatrix commented Jun 19, 2018

@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
Copy link
Collaborator

KevinOConnor commented Jun 19, 2018 via email

@mcmatrix
Copy link
Contributor Author

mcmatrix commented Jun 19, 2018

@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
Copy link
Collaborator

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

-Kevin

@mcmatrix
Copy link
Contributor Author

mcmatrix commented Jun 20, 2018

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:

  • Each named button is separate queue with size 1
  • Implement thread locking for accessing last_pressed. But thread locking for whole list, it blocks access for all buttons.
  • Separate lock for each named button?

@mcmatrix
Copy link
Contributor Author

mcmatrix commented Jun 20, 2018

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
Copy link
Contributor Author

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
Copy link
Contributor Author

mcmatrix commented Jun 21, 2018

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:

  • work-display-20180620
    • klippy/extras/display.py
  • work-menu-20180616
    • klippy/extras/menu.py
  • work-buttons-20180620 (based on original lcd branch)
    • klippy/extras/buttons.py
    • other files

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
Copy link

hg42 commented Jun 22, 2018

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:

  • a submenu could be entered via a button field jumping to another page (may be type "jump" or "page")
  • a gcode command would also be a button field (may be type "command" or "gcode")

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
Copy link
Contributor Author

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
Copy link
Contributor Author

mcmatrix commented Jun 22, 2018

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
Copy link
Collaborator

KevinOConnor commented Jun 22, 2018 via email

@mcmatrix
Copy link
Contributor Author

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
Copy link

Taz8du29 commented Jun 23, 2018

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:

  • Reduces main config file size
  • Menu hierarchy is easier to understand
  • Can be reused across configs
  • Easy to parse using python dictionnaries

@micheleamerica
Copy link

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
Copy link
Contributor Author

mcmatrix commented Jun 24, 2018

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
Copy link

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
Copy link
Contributor Author

mcmatrix commented Jun 24, 2018

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
Copy link

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
Copy link
Contributor Author

mcmatrix commented Jun 24, 2018

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
Copy link

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
Copy link
Contributor Author

mcmatrix commented Jun 24, 2018

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.

@mcmatrix mcmatrix changed the title Basic menu support (WIP) (DRAFT) Basic menu support (beta) Jun 24, 2018
@micheleamerica
Copy link

micheleamerica commented Jun 25, 2018

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
Copy link
Contributor Author

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

@mcmatrix
Copy link
Contributor Author

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

@micheleamerica
Copy link

Great news. Thanks.

@micheleamerica
Copy link

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
Copy link
Contributor Author

@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?

@bruce356
Copy link

@mcmatrix please find attached my printer.cfg file
regards
printer.zip

@mcmatrix
Copy link
Contributor Author

mcmatrix commented Jul 22, 2018

@bruce356 To use menu you have to enable it in display section
First you have to find out what pins to use for buttons and encoder.

In example-extras.cfg

# Support for a display attached to the micro-controller.
#[display]
#lcd_type:
#   The type of LCD chip in use. This may be "hd44780" (which is
#   used in "RepRapDiscount 2004 Smart Controller" type displays),
#   "st7920" (which is used in "RepRapDiscount 12864 Full Graphic
#   Smart Controller" type displays) or "uc1701" (which is used in
#   "MKS Mini 12864 type" displays). This parameter must be provided.
#rs_pin:
#e_pin:
#d4_pin:
#d5_pin:
#d6_pin:
#d7_pin:
#   The pins connected to an hd44780 type lcd. These parameters must
#   be provided when using an hd44780 display.
#cs_pin:
#sclk_pin:
#sid_pin:
#   The pins connected to an st7920 type lcd. These parameters must be
#   provided when using an st7920 display.
#cs_pin:
#a0_pin
#   The pins connected to an uc1701 type lcd. These parameters must be
#   provided when using an uc1701 display.
#menu_root:
#   Entry point for menu, root menuitem name. This parameter must be 
#   provided when using menu.
#menu_timeout:
#   Timeout for menu. Being inactive this amount of seconds will trigger 
#   menu exit or return to root menu when having autorun enabled [optional]
#menu_autorun:
#   Enable this to run menu immediately at startup. It's mostly used to replace
#   stock info screens with custom ones. It accepts boolean values, 
#   default is False. [optional]
#rows:
#   Only needed when using display with custom row count [Optional].
#cols:
#   Only needed when using display with custom column count [Optional].
#encoder_pins:
#   The pins connected to encoder. 2 pins must be provided when 
#   using encoder. This parameter must be provided when using menu.
#click_pin:
#   The pin connected to 'enter' button or encoder 'click'. This parameter
#   must be provided when using menu.
#back_pin:
#   The pin connected to 'back' button [Optional].
#up_pin:
#   The pin connected to 'up' button. This parameter must be provided 
#   when using menu without encoder.
#down_pin:
#   The pin connected to 'down' button. This parameter must be provided 
#   when using menu without encoder.
#kill_pin:
#   The pin connected to 'kill' button. This button will call 
#   emergency stop.

This is my test config for display section

[display]
lcd_type: hd44780
rs_pin: ar16
e_pin: ar17
d4_pin: ar23
d5_pin: ar25
d6_pin: ar27
d7_pin: ar29
#menu_root: infoscreen
menu_root: main
#menu_autorun: yes
#menu_timeout: 60
encoder_pins: ^!ar31, ^!ar33
click_pin: ^!ar35
kill_pin: ^!ar41

@mcmatrix
Copy link
Contributor Author

Added initial draft description of menu element attributes to example-menu.cfg

@Zwaubel
Copy link

Zwaubel commented Jul 22, 2018

@mcmatrix nice work of yours!

I noticed two minor issues regarding the case light control in your example menu:

  1. When switching the case light on or off, the menu cursor changes its position right after the gcode was executed.
    In my case it jumps from "Case Light" to "Move 1mm".

  2. I can't set a specific output pin value for the case light pin. When changing the value and confirming by pushing the encoder rod, the value jumps right back to what it was before. No change in the case light pwm or anything.

config atteched:
printer.zip

@mcmatrix
Copy link
Contributor Author

mcmatrix commented Jul 22, 2018

Should be fine now.
Please check example-config there was a mistake in caselight gcode.

I had to remove realtime menuitems enable/disable update.
When executing gcode klipper goes for a couple of seconds to the printing state
and it was causing menuitems hide/show and cursor jump.

Now items enable/disable status is only updated when entering to menu or submenu.

@Zwaubel
Copy link

Zwaubel commented Jul 22, 2018

Great! Works fine now, thank you!

@bruce356
Copy link

@mcmatrix thanks very much for your efforts to help me.
I am complete novice and have been trying to get # "RepRapDiscount 128x64 Full Graphic Smart Controller" working for many hours but this pins thing is so confusing (China Rumba board). Why is "^!" placed before the pin number.
I will wait for someone with more knowledge to get this type of GLCD working.
regards - bruce

@Taz8du29
Copy link

@bruce356 The "^!" syntax is described in the example.cfg configuration file:

Pin names may be preceded by an '!' to indicate that a reverse polarity should be used (eg, trigger on low instead of high).
Input pins may be preceded by a '^' to indicate that a hardware pull-up resistor should be enabled for the pin.

You may have to look at the datasheet for your screen in order to determine the pinout of your LCD.

If I understood well, you have this LCD screen. The reprap wiki page gives you the following schematics, and you can find the config for your LCD and board in the Marlin firmware example.

If you don't know how the pins have to be trigerred (high or low), you can just experiment with the config... as long as the wiring is fine, there is no risk for the hardware :)

@bruce356
Copy link

bruce356 commented Jul 24, 2018

@Taz8du29, Thanks for the information, you inspired me to try again.
This is my final result
RepRapDiscount 128x64 Full Graphic Smart Controller
[display]
lcd_type: st7920
cs_pin: ar19
sclk_pin: ar18
sid_pin: ar42
menu_timeout: 60
menu_root: main
menu_autorun: yes
encoder_pins: ^ar11, ^ar12
click_pin: ^ar43
kill_pin: ^!ar46

The kill_pin just seems to lock up the system (I expected a reset as in Marlin) but its not required very often so does not matter.

@mcmatrix this is a fantastic result thank you very much.
regards - bruce

@mcmatrix
Copy link
Contributor Author

@KevinOConnor What you think? Should we wait for more testings (It's a summertime and people are away from computers) or I'll make PR with 2 commits (for display modifications and menu stuff) and lets merge it with master.

@KevinOConnor
Copy link
Collaborator

KevinOConnor commented Jul 25, 2018 via email

@bruce356
Copy link

@mcmatrix , hi just a thought, seeing how you are a good knowledgeable programmer (only if you have the spare time) had you given any thoughts to mesh bed levelling for Klipper using a probe like the BLTouch. This would make Klipper far more attractive for larger printer bed users.
regards - bruce

@mcmatrix
Copy link
Contributor Author

@bruce356 Thank you. There are more talented programmers than me :)
I'm having delta with glass bed so mesh leveling is quite useless for me.
If my bed is somehow transformed then my delta calibration will be off anyway.
Don't worry, if the Klippers community will grow then all needed features will come sooner or later ;)

@bruce356
Copy link

bruce356 commented Aug 1, 2018

@mcmatrix , hi would it be possible to add "Babystepping" to the Z axis to fine tune the first layer offset from the bed (maybe in 0.005 increments). I understand that KevinOConnor has added M206 to SET_GCODE_OFFSET for fine adjustment. When starting a print it is much easier quicker to make that final fine adjustment from the GLCD.

Another minor point, under the OctoPrint heading (using your menu) there are no sub options, not sure why.
Regards - bruce

klipper mcmatrix menu

printer.zip

@mcmatrix
Copy link
Contributor Author

mcmatrix commented Aug 1, 2018

Another minor point, under the OctoPrint heading (using your menu) there are no sub options, not sure why.

If you examine Octoprint menu items more closely

### menu octoprint ###
[menu octoprint]
type: list
name: OctoPrint
items: 
    octoprint_pause
    octoprint_resume
    octoprint_abort

[menu octoprint_pause]
type: command
enable: toolhead.is_printing
name: Pause printing
gcode: ECHO MSG=@pause

[menu octoprint_resume]
type: command
enable: toolhead.is_printing
name: Resume printing
gcode: ECHO MSG=@resume

[menu octoprint_abort]
type: command
enable: toolhead.is_printing
name: Abort printing
gcode: ECHO MSG=@abort

then there's an attribute enable: toolhead.is_printing
it means that menu item is only enabled (visible) when this condition is true.
If you want these items visible all the time then just remove enabled attribute
or put it enable: true

hi would it be possible to add "Babystepping" to the Z axis to fine tune the first layer offset from the bed (maybe in 0.005 increments). I understand that KevinOConnor has added M206 to SET_GCODE_OFFSET for fine adjustment. When starting a print it is much easier quicker to make that final fine adjustment from the GLCD.

SET_GCODE_OFFSET [X=<pos>|X_ADJUST=<adjust>] [Y=<pos>|Y_ADJUST=<adjust>] [Z=<pos>|Z_ADJUST=<adjust>]: Set a positional offset to apply to future G-Code commands. This is commonly used to virtually change the Z bed offset or to set nozzle XY offsets when switching extruders. For example, if "SET_GCODE_OFFSET Z=0.2" is sent, then future G-Code moves will have 0.2mm added to their Z height. If the X_ADJUST style parameters are used, then the adjustment will be added to any existing offset (eg, "SET_GCODE_OFFSET Z=-0.2" followed by "SET_GCODE_OFFSET Z_ADJUST=0.3" would result in a total Z offset of 0.1).

I think it's possible. But I leave it for you to figure it out.
Please look existing items in example-menu.cfg
You should be able to put this together quite easily. Hint, it should be input type, input parameter should be number (0), in gcode you should use Z_ADJUST. So whatever amount you will execute it will be added to existing offset. You cannot read back existing offset in menu item, so each time you start this "babystepping" menu input starts from 0.

@bruce356
Copy link

bruce356 commented Aug 2, 2018

@mcmatrix, I appreciate what you are trying to do but I have no clue to programming in Python or anything else. It is easy to look at what other people have done and copy plus make minor changes.

I have used your Test Menu section to attempt to add a BabyStepping section as follows:-

test menu

[menu test]
type: input
enable: true
name: "BabySt Z:{0:05.3f}"
parameter: toolhead.zpos
input_min: -0.2
input_max: 0.25
input_step: 0.005
gcode: G1 Z{0:.3f}

I have no idea if this is what you had in mind but it is just about all I can do to copy your example and make minor changes.

Thanks and regards - bruce

@mcmatrix
Copy link
Contributor Author

mcmatrix commented Aug 2, 2018

I would try something like these

[menu babystep_adj]
type: input
name: "BabySt adj Z:{0:05.3f}"
parameter: 0
input_min: -0.25
input_max: 0.25
input_step: 0.005
gcode: SET_GCODE_OFFSET Z_ADJUST={0:.3f}

[menu babystep_abs]
type: input
name: "BabySt abs Z:{0:05.3f}"
parameter: 0
input_min: -0.25
input_max: 0.25
input_step: 0.005
gcode: SET_GCODE_OFFSET Z={0:.3f}

babystep_abs is for setting absolute offset value and babystep_adj will adjust existing offset by amount.

At the moment there's no way of reading these offset current values in menu input so you have to use 0 as an initial value.
In the terminal, you can check existing offsets when using GET_POSITION gcode and look gcode homing: section.

@bruce356
Copy link

bruce356 commented Aug 2, 2018

@mcmatrix , thanks very nicely done and nothing like I imagined.
I have finally uploaded all to my Dual X Carriage printer.

What I find strange is that none of the movement settings through the GLCD take effect until dial button is pressed. Is it not possible for movement to take place while rotating the dial as with other firmware.

The Babystepping does work but with a considerable time delay (I guess it depends on where it was placed in the Que) and also no effect until after button press.
Regards - bruce

@mcmatrix
Copy link
Contributor Author

mcmatrix commented Aug 2, 2018

What I find strange is that none of the movement settings through the GLCD take effect until dial button is pressed. Is it not possible for movement to take place while rotating the dial as with other firmware.

This is how input item is currently working. Click on edit item will enter into edit mode allowing value change and another click will exit from this mode and formatted gcode will be executed.
Could be that in the future this will be modified to allow realtime change.

@bruce356
Copy link

bruce356 commented Aug 3, 2018

@mcmatrix , Just going through and doing some testing of the Menu Move system, X Y & Z seem to be working as expected but "E" is making strange movements on my printer, movements seem to be very fast (that may be by design, can this be changed), nothing above 50mm of movement seems to work for me and the movement distance is not consistent, sometimes moving the set distance other times just a small amount and other times no movement at all. Is the movement on "E" set to G91 Relative Positioning or G90 Absolute Positioning (can this be changed from one to the other).

I am very pleased with this addition to Klipper, without the GLCD Menu option I would not have gone from Marlin to Klipper as I prefer to control my printer this way.

Thanks & regards - bruce
PS it seems that "E" is set to G90 Absolute Positioning but does not show its actual position as for XYZ this is confusing, would it not be better to have "E" set to G91 Relative Positioning?

@Taz8du29
Copy link

Taz8du29 commented Aug 5, 2018

@bruce356 For the 50mm distance, it's normal. If you didn't change the configuration, Klipper defaults the maximum allowed extruder move to 50mm.

Ses the example.cfg file documentation for more details

@bruce356
Copy link

bruce356 commented Aug 6, 2018

@Taz8du29 , thanks I missed that option, so many things to configure.
@mcmatrix , I understand now how extruder move functions (as you have set it up) but as it is, it would be nice to display its actual position from the starting point (starting point being +000.0).
If moved to say +100mm (& once moved the display returns to +000.0) and it may need reversing by say 10mm that then means setting the display to +90mm (& again once moved the display returns to +000.0).
My logical assumption was that if wanting to reverse by 10mm the display would need to be set at -010.0 but this causes the extruder to reverse by 110mm not the 10mm expected.
That is why I feel the display should show actual position (if its possible on the extruder) otherwise one has to remember ones last position selection, other wise unpredictable movement will occur.
The other option would be to set the extruder (if its possible) for that short period of use to G91 Relative Positioning.
Regards -bruce

@mcmatrix
Copy link
Contributor Author

mcmatrix commented Aug 6, 2018

@bruce356 Thanks for your feedback.
I'll check once more how extruder movements are working.

@unnefer1
Copy link

unnefer1 commented Aug 6, 2018

I love the work you've done, very nice and great effort.

My only feedback, which has most likely come too late now, is a change to the naming convention you've used for the sections in printer.cfg and how the root items are initially set - all in the name of making it more user friendly and easier to navigate.

Due to the current naming convention, if a very descriptive group or list item name is not used, and/or the sections are not entered in a flowing order, it makes it all but impossible to quickly discern what section belongs to what.

On quick inspection, everything basically looks like it belongs to a single menu item called "menu" as each section lists the word "menu" first and then only the group or list item name.

Adding to the problem above, if there are any instances it would make sense to use the same group or list item name (eg. up, down, on, off, etc), you cannot actually do that with the current naming convention as you'd end up with a duplicate section name, so you need to append an index of some sort so there is no conflict (eg. up1, down1, up2, down2, on1, off1, on2, off2, etc).

Current menu structure of a pretty standard menu for a 3D printer:

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

[menu main1]
type: group
name: Main menu
items: info, control, octoprint, babysteps

[menu info]	
type: list
name: Information
items: version, status

[menu version]
type: command
...

[menu status]
type: command
...

[menu control]	
type: group
name: Printer Control
items: fan, temperature, movement, 

[menu fan]
type: list
name: Fan Control
items: 
    on
    off
...

[menu on]
type: command
...

[menu off]
type: command
...

[menu temperature]
type: group
name: Temperature
items: bed, nozzle

[menu bed]
type: list
name: Adjust Bed Temperature
items: 
    down
    up
    off2
...

[menu down]
type: command
...

[menu up]
type: command
...

[menu off2]
type: command
...
[menu nozzle]
type: list
name: Adjust Nozzle Temperature
items: 
    down2
    up2
    off3
...

[menu down2]
type: command
...

[menu up2]
type: command
...

[menu off3]
type: command
...

[menu movement]
type: group
name: Move Axis
items: X, Y, Z
...

[menu X]
type: list
name: Move X Axis
items: 
    left
    right

[menu left]
type: command
...

[menu right]
type: command
...

[menu Y]
type: list
name: Move Y Axis
items: 
    back
    forward

[menu back]
type: command
...

[menu forward]
type: command
...

[menu Z]
type: list
name: Move Z Axis
items: 
    down4
    up4

[menu down4]
type: command
...

[menu up4]
type: command
...

[menu octoprint]
type: list
name: OctoPrint
items: 
    pause
    resume
    abort

[menu pause]
type: command
...

[menu resume]
type: command
...

[menu abort]
type: command
...

[menu babysteps]
type: list
name: BabySteps
items: 
    adj
    abs

[menu adj]
type: input
...

[menu abs]
type: input
...

My suggestion, as can be seen below, is to use a parent/child approach for the naming convention of the sections, which makes it far quicker and easier to identify what parent group/list section each child item section belongs to.

By naming the sections the way I have below, it gets around the problems where list items use the same name (eg. up, down, on, off, etc) as the section name includes the parent group/list name, meaning you don't have to append an index - and that makes it very easy to work through and know exactly what belongs to what.

The same menu structure as above, except this time with the cvhanges I suggest:

# Menu manager
[menu]
rows: 4
cols: 16
type: group
name: Main Menu
items: info, control, octoprint, babysteps 
#Root "parent" menu items above are listed on LCD when menu is activated

[menu info]	
type: list
name: Information
items: version, status

[menu info version]
type: command
...

[menu info status]
type: command
...

[menu control]	
type: group
name: Printer Control
items: fan, temperature, movement, 

[menu control fan]
type: list
name: Fan Control
items: 
    on
    off
...

[menu control fan on]
type: command
...

[menu control fan off]
type: command
...

[menu control temperature]
type: group
name: Temperature
items: 
    bed
    nozzle

[menu control temperature bed]
type: list
name: Adjust Bed Temperature
items: 
    down
    up
    off
...

[menu control temperature bed down]
type: command
...

[menu control temperature bed up]
type: command
...

[menu control temperature bed off]
type: command
...

[menu control temperature nozzle]
type: list
name: Adjust Nozzle Temperature
items: 
    down
    up
    off
...

[menu control temperature nozzle down]
type: command
...

[menu control temperature nozzle up]
type: command
...

[menu control temperature nozzle off]
type: command
...

[menu control movement]
type: group
name: Move Axis
items: X, Y, Z
...

[menu control movement X]
type: list
name: Move X Axis
items: 
    left
    right

[menu control movement X left]
type: command
...

[menu control movement X right]
type: command
...

[menu control movement Y]
type: list
name: Move Y Axis
items: 
    back
    forward

[menu control movement Y back]
type: command
...

[menu control movement Y forward]
type: command
...

[menu control movement Z]
type: list
name: Move Z Axis
items: 
    down
    up

[menu control movement Z down]
type: command
...

[menu control movement Z up]
type: command
...

[menu octoprint]
type: list
name: OctoPrint
items: 
    pause
    resume
    abort

[menu octoprint pause]
type: command
...

[menu octoprint resume]
type: command
...

[menu octoprint abort]
type: command
...

[menu babysteps]
type: list
name: BabySteps
items: 
    adj
    abs

[menu babysteps adj]
type: input
...

[menu babysteps abs]
type: input
...

@mcmatrix
Copy link
Contributor Author

mcmatrix commented Aug 7, 2018

@unnefer1 You have an interesting concept by using namespaces or relative naming.
I added a small change in menu code so that you can use a mixed namings :)

### menu main ###
[menu main]
type: list
name: Main Menu
items:
    octoprint

### menu octoprint ###
[menu octoprint]
type: list
name: OctoPrint
items:
    .pause
    .resume
    octoprint_abort
    octoprint abort2

[menu octoprint pause]
type: command
enable: toolhead.is_printing
name: Pause printing
gcode: ECHO MSG=@pause

[menu octoprint resume]
type: command
enable: toolhead.is_printing
name: Resume printing
gcode: ECHO MSG=@resume

[menu octoprint_abort]
type: command
enable: toolhead.is_printing
name: Abort printing
gcode: ECHO MSG=@abort

[menu octoprint abort2]
type: command
enable: toolhead.is_printing
name: Abort printing2
gcode: ECHO MSG=@abort

When in attribute items the item name starts with . then the system will add container namespace as a prefix to it otherwise absolute naming is used.
It's still in my local repo. It'll be available when I push PR update.
Stay tuned!

@unnefer1
Copy link

unnefer1 commented Aug 8, 2018

That is really awesome Janar, I didn't expect any change would be made this far along. Top stuff.

I'm working through a few prints at the moment, so won't have the chance to pull the code until the weekend, but once I have a moment, I'll give it a go using attirbute items and spacing in the sections names.

Cheers.

@mcmatrix
Copy link
Contributor Author

mcmatrix commented Aug 8, 2018

PR #500 is updated with the latest work.

@mcmatrix
Copy link
Contributor Author

This feature is now in the master branch.

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

No branches or pull requests