Reprapworld Keypad: Prevent key-mashing issues?

[thinkyhead]

Addressing #3915

Pressing the Home key while also pressing a movement key can cause an axis to run out of control. This patch attempts to address the problem by preventing re-entrant handling of keypad moves, based on the speculation that this is the main reason for the anomalous behavior.

Additionally, this patch reduces the code for implementing keypad-based axis movement.

Download ZIP: https://github.com/thinkyhead/Marlin/archive/rc_reprapworldkeypad_tweaks.zip

[maukcc]

The only things that need some work are:

• home commands are being executed while printing
• move step is being set by REPRAPWORLD_KEYPAD_MOVE_STEP.
  I like that it is being set for default by REPRAPWORLD_KEYPAD_MOVE_STEP, but you can not use the Move axis menu now

[maukcc]

Also : if I press stop on the display it stops for half a second and then continues.
If I then also press home on the keypad it gets stuck in a homing loop

[maukcc]

The silly stop behaviour is not affected by disabling the keypad. so that has some other cause.

[thinkyhead]

home commands are being executed while printing

There's no definite method for Marlin to check for "is printing" during host printing, only during SD printing, so it's not easy to disable the "home" button based on that. So I suppose it does have to be disabled based on the axes already being homed with G28….

[thinkyhead]

if I press stop on the display it stops for half a second and then continues

Do you mean that it does this during manual moves from the LCD, or if you try to stop a print in progress?

[thinkyhead]

home on the keypad it gets stuck in a homing loop

Probably just keeps adding more homing commands. Do the keys behave in such a way that you have to repeatedly tap them to get movement, or can you hold them down? It seems to me we should treat keypad events more like debounced keyboard (or button) events and less like generic controller-knob events.

[maukcc]

if I press stop on the display it stops for half a second and then continues

It is when a print is in progress.

home on the keypad it gets stuck in a homing loop

this is only after a stop print is done(as above), and it is continuing its print.

You can hold the key down, and it will do steps, for 10mm increments it will eventually do 1 move and not X steps

[maukcc]

I just tested the stop sd print behaviour on a "fresh" install and it happens also. new code in RCbugfix.
Probably better to make a new entry for this?

[thinkyhead]

It is when a print is in progress.
only after a stop print is done(as above), and it is continuing its print

When you use "Stop print" it should turn off all the heaters, kill SD printing, and show "Print aborted" on the LCD screen. I can't see any reason why Marlin would keep reading from the SD card after that point.

I've made some additional changes to this PR (and branch):

• The Home button only works when the machine isn't homed already.
• The keypad now debounces. You must release all keys before you can depress another.
• Keypad move distance is now 1mm by default.

So give this a test and make sure it won't allow homing in the middle of a print.

The "Stop print" problem should be reported as a separate issue.

[thinkyhead]

@maukcc Looking forward to your feedback as soon as you can get this tested.

[maukcc]

OK, I am back to RCbugfix :)
I am missing in ultralcd.h

#define REPRAPWORLD_KEYPAD_MOVE_MENU    (buttons_reprapworld_keypad & EN_REPRAPWORLD_KEYPAD_F1)

G28 command does silly things (move X,Y and Z 10mm in + direction)

when using the keypad now and I press and hold the button, it will not do anything until I release the button and then it moves Xmm. It used to move while I held the button down and stop when released(much better)

It uses the default REPRAPWORLD_KEYPAD_MOVE_STEP which I (as user) want to control.

I cant test more as it does not home

[thinkyhead]

Looking forward to your feedback
OK, I am back to RCbugfix :)

Instead of RCBugFix you should be testing this PR. The code is at https://github.com/thinkyhead/Marlin/tree/rc_reprapworldkeypad_tweaks

[maukcc]

OK, using that PR
1st thing was:

static void reprapworld_keypad_move_menu() { lcd_goto_menu(lcd_move_menu); }

has to be:

static void reprapworld_keypad_move_menu() { lcd_goto_screen(lcd_move_menu); }

but then the LCD did not work so I can not test further.

[thinkyhead]

Sorry to hear the LCD is acting up. I've made the change you pointed out. Test again as soon as you have a working LCD.

[maukcc]

My LCD is working fine. It is just not working with this firmware :)

[thinkyhead]

My LCD is working fine. It is just not working with this firmware

@maukcc In what way is it not working? Are you able to make any tweaks or experiment with different options to get it working? What kind of LCD is it?

[thinkyhead]

Calling in testers from various online RepRap groups…. Let's get this thing tested and merged!

[maukcc]

My display was left out :)

#define HAS_LCD_CONTRAST ( \
    ENABLED(MAKRPANEL) \
 || ENABLED(CARTESIO_UI) \
 || ENABLED(VIKI2) \
 || ENABLED(miniVIKI) \
 || ENABLED(ELB_FULL_GRAPHIC_CONTROLLER) \
)

• I can not change the mm per click(lcd_move_menu), as this is already set by

  #define REPRAPWORLD_KEYPAD_MOVE_STEP 1.0

• When moving, a click(no matter how long) moves it 1 step. previously it kept going as long as you pressed the button
• when a move menu entry is made the display goes back to lcd_move_menu_axis, it should go back to status_screen.
• Or better: go to a screen that displays X,Y and Z values

[thinkyhead]

• I can not change the mm per click (lcd_move_menu), as this is already set by REPRAPWORLD_KEYPAD_MOVE_STEP.

How would you propose we change this? Do you want an option to change the distance per click from the menu?

• When moving, a click (no matter how long) moves it 1 step. previously it kept going as long as you pressed the button.

That was absolutely my intention. I'm glad to hear that it's working as intended. From here I will need to work out a reasonable method to have an axis continue to move while the key is held down. But for now, this is what I wanted.

• When a move menu entry is made the display goes back to lcd_move_menu_axis, it should go back to status_screen. Or better: go to a screen that displays X,Y and Z values.

Perhaps. I will have a look at this.

Did you try pressing multiple keys at once to see how it behaves?

[maukcc]

How would you propose we change this? Do you want an option to change the distance per click from the menu?

This is where the F1(move menu) button is/was for? unless:

That was absolutely my intention. I'm glad to hear that it's working as intended. From here I will need to work out a reasonable method to have an axis continue to move while the key is held down. But for now, this is what I wanted.

if you are making it: the longer you press the button the faster it goes. and every step is 0.1.
Then there is no need for a move menu and we can use the F1 button for something else.
Like setting current position to zero (for lasering or milling or whatever), This may not be done while printing of coarse.

Did you try pressing multiple keys at once to see how it behaves?

I did not, but have now . I can not get it to do silly things :)

[thinkyhead]

I like your interface idea. Tap to move one small step, but hold down for a continuous, moderately accelerating, move after a half-second or so. I could do that. (Another option is to use a fast double-tap for 10mm, or a single tap for 1mm, removing the need to tap 10 times, or hold and wait, to move 10mm.)

[maukcc]

Here is the outcome of the serial debug
I did:

1. - home button
2. - 5x 1 click X axis (+ direction)
3. - 1x 5sec long click X axis (+ direction)
4. - 5x 1 click Y axis (+ direction)
5. - 1x 5sec long click Y axis (+ direction)

Connecting...
start
Printer is now online.
echo: External Reset
Marlin 1.1.0-RCBugFix
echo: Last Updated: 2016-04-27 12:00 | Author: (none, default config)
Compiled: Jul 20 2016
echo: Free Memory: 3472  PlannerBufferBytes: 1232
echo:Hardcoded Default Settings Loaded
echo:Steps per unit:
echo:  M92 X71.13 Y71.13 Z640.00 E152.00
echo:Maximum feedrates (mm/s):
echo:  M203 X200.00 Y200.00 Z20.00 E20.00
echo:Maximum Acceleration (mm/s2):
echo:  M201 X1000 Y1000 Z100 E10000
echo:Accelerations: P=printing, R=retract and T=travel
echo:  M204 P500.00 R10000.00 T1000.00
echo:Advanced variables: S=Min feedrate (mm/s), T=Min travel feedrate (mm/s), B=minimum segment time (ms), X=maximum XY jerk (mm/s),  Z=maximum Z jerk (mm/s),  E=maximum E jerk (mm/s)
echo:  M205 S0.00 T0.00 B20000 X10.00 Z0.40 E5.00
echo:Home offset (mm)
echo:  M206 X0.00 Y0.00 Z0.00
echo:Material heatup parameters:
echo:  M145 S0 H180 B70 F0
echo:  M145 S1 H240 B110 F0
echo:PID settings:
echo:  M301 P22.20 I1.08 D114.00 C100.00 L20
echo:LCD Contrast:
echo:  M250 C90
echo:Filament settings: Disabled
echo:  M200 D3.00
echo:  M200 D0
echo:SD card ok
echo:SD card ok
echo:enqueueing "G28"
echo:busy: processing
Enter loop : move_menu_scale=1.00, next_move_ms=19024, repeat_ms=250
still looping for axis 0
Exit loop : move_menu_scale=0.10, next_move_ms=19280, repeat_ms=100
Enter loop : move_menu_scale=1.00, next_move_ms=21482, repeat_ms=250
still looping for axis 0
Exit loop : move_menu_scale=0.10, next_move_ms=21737, repeat_ms=100
Enter loop : move_menu_scale=1.00, next_move_ms=24154, repeat_ms=250
still looping for axis 0
Exit loop : move_menu_scale=0.10, next_move_ms=24409, repeat_ms=100
Enter loop : move_menu_scale=1.00, next_move_ms=25107, repeat_ms=250
Exit loop : move_menu_scale=1.00, next_move_ms=25107, repeat_ms=250
Enter loop : move_menu_scale=1.00, next_move_ms=26039, repeat_ms=250
still looping for axis 0
Exit loop : move_menu_scale=0.10, next_move_ms=26294, repeat_ms=100
Enter loop : move_menu_scale=1.00, next_move_ms=27976, repeat_ms=250
still looping for axis 0
Exit loop : move_menu_scale=0.10, next_move_ms=28231, repeat_ms=100
Enter loop : move_menu_scale=1.00, next_move_ms=69670, repeat_ms=250
still looping for axis 1
Exit loop : move_menu_scale=0.10, next_move_ms=69926, repeat_ms=100
Enter loop : move_menu_scale=1.00, next_move_ms=70628, repeat_ms=250
still looping for axis 1
Exit loop : move_menu_scale=0.10, next_move_ms=70883, repeat_ms=100
Enter loop : move_menu_scale=1.00, next_move_ms=71428, repeat_ms=250
still looping for axis 1
Exit loop : move_menu_scale=0.10, next_move_ms=71683, repeat_ms=100
Enter loop : move_menu_scale=1.00, next_move_ms=72224, repeat_ms=250
still looping for axis 1
Exit loop : move_menu_scale=0.10, next_move_ms=72480, repeat_ms=100
Enter loop : move_menu_scale=1.00, next_move_ms=72882, repeat_ms=250
still looping for axis 1
Exit loop : move_menu_scale=0.10, next_move_ms=73137, repeat_ms=100
Enter loop : move_menu_scale=1.00, next_move_ms=73694, repeat_ms=250
still looping for axis 1
Exit loop : move_menu_scale=0.10, next_move_ms=73949, repeat_ms=100

[thinkyhead]

@maukcc From the log it appears that the keypad's buttons cannot be held down. The key bits appear to register a brief "on" signal then clear themselves until the next time you tap the key. So, no key repeat is possible with this hardware.

Just to be sure, I've sent an email to ask whether there's some way to get persistent bits.

[maukcc]

This is how it works in RC6:
https://youtu.be/U7HyiK5pB_s
so you can hold the button down and get repeated steps.
this works fine, but the way we are trying it now will be better :)

[thinkyhead]

@maukcc Hmm! I am surprised to see the key "repeating." The loop in the new code is very explicit about continuing for as long as any key is held down, and yet it exits the loop. Between your video of RC6 and the results of the log it looks lot like the keypad is continually sending on-off-on-off for the pressed key. But just to be sure, I've added even more logging to show us the actual keypad bits as they change. I also went one level up and added logging for the initial key press.

After you test with this logging, we will know much more about how this keypad actually works.

[maukcc]

Here is the outcome of the new serial debug
I did:

- home button
- 5x 1 click X axis (+ direction)
- 1x 5sec long click X axis (+ direction)

test.txt

[thinkyhead]

The bit logging was a little strange that time. I intended that it should only output the bits when they change. Maybe that part was working properly, but I made an error in not using a static string to store the hex output, so the printed values are bogus. Now that I've patched it up properly, please run the new code once more! Try the X, Y, and Z buttons so we get a good sampling of the bits.

[thinkyhead]

I got a reply from tech support:

That's a good question, and I don't have an immediate answer for
you. Our keypad architect is on travel this week and will be back
this weekend. He should be able to give you an answer early next
week on the keypad persistence.

… and in reply to my saying we'd keep working on it …

Thanks for being proactive on this question. Send an update to our
techsupport line if you make progress this weekend. Otherwise we'll
let you know what we find.

[thinkyhead]

Further info from Bart Meijer:

The keypad will register a press until release. Anything else is software messing with you. Can you give a more elaborate scenario of what you are trying to do?

I have described our scenario, linked to this code, and provided our shift-register reading code for additional feedback.

[maukcc]

Here is the outcome of the new serial debug
What I also saw was that "X" amount of rapid clicks did not do Xmm of movement, but only a movement now and then. So the clicks in the files had at least 1 sec in between them.

testX file:

• home button
• 5x 1 click X axis (+ direction)
• 1x 5sec long click X axis (+ direction)
• 5x 1 click X axis (- direction)
• 1x 5sec long click X axis (- direction)

testY file:

• home button
• 5x 1 click Y axis (+ direction)
• 1x 5sec long click Y axis (+ direction)
• 5x 1 click Y axis (- direction)
• 1x 5sec long click Y axis (- direction)

testZ file:

• home button
• 5x 1 click Z axis (+ direction)
• 1x 5sec long click Z axis (+ direction)
• 5x 1 click Z axis (- direction)
• 1x 5sec long click Z axis (- direction)

testMoveMenu file:

• 1x 1 click Move menu button
• 1x 5sec long click Move menu button

testX.txt
testY.txt
testZ.txt
testMoveMenu.txt

[thinkyhead]

Darn-it, I'm sorry. Those HEX numbers are still screwy. There should only be 2 digits, for one thing. I need to debug the debugging code. How ironic.

I got another reply from the maker of the keypad. He sent a piece of code that confirms we are reading the keypad the correct way. So, now I just have to get that hex output working correctly, and we can do one more test…

[maukcc]

We will be closed for holidays in August.
If you want I can sent you a User interface to test and use.
Make an account on our webshop, so I have your address.

[thinkyhead]

@maukcc Thanks! That will help a lot. I really want to make this keypad work as well as possible.

I'm unable to create an account because the form requires impertinent information, and I don't currently have an address of my own anyway, so you will have to ship care-of my friend's studio. Send me your email address using my contact form and I will send you my shipping address.

[maukcc]

Did you receive the UI?
I tried latest bugfix, but had no succes with the keypad.
Or am I missing some settings?

[thinkyhead]

@maukcc Unfortunately for the UI I've been in Austin, Texas since mid-August, but I'm heading back to Portland next week. I've got to settle some affairs while I'm there, but I'm sure I will have plenty of spare time to work with the UI too.

[maukcc]

Did you have a chance to test the UI with the keypad?