MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.24k stars 19.22k forks source link

Encoder selecting wrong menu item (RepRapDiscount Full Graphics Smart Controller) #10306

Closed marcio-ao closed 6 years ago

marcio-ao commented 6 years ago

@thinkhead, first off, thank you so much for commit ac4fac7 which fixes the LCD highlight glitch that a lot of our users had been complaining about. That fix is awesome!

Alas, even with that out of the way, some folks have been reporting that they were still mis-selecting menu items. This happens sporadically, but I think I may have found a way to reproduce it:

This is fairly reproducible once you know how to trigger the effect and it happens in the bugfix-2.0.x. During normal use, this happens quite a bit by accident, probably because depending on the position of the user's finger on the knob, they may inadvertently nudge the knob half way to the next detent while pushing down.

Since in such cases the highlight is not changed, the user may blame Marlin for doing something wonky. The fact that it seemingly happens randomly is frustrating.

Here are our encoder config:

#define ENCODER_PULSES_PER_STEP 2
#define ENCODER_STEPS_PER_MENU_ITEM 1

Let me know if you are able to reproduce this.

Thanks!

-- Marcio

thinkyhead commented 6 years ago

Yes, I've seen this kind of issue. It's sporadic. The highlight may not be drawing because it isn't setting lcdDrawUpdate at just the right magic moment, or the act of pressing the encoder button may be causing the encoder to slip. The only thing we can do is add debug messages to ultralcd.cpp and spend some hours troubleshooting until we figure out the exact cause. Maybe it will be enough to quantize the encoder value to ENCODER_PULSES_PER_STEP on each iteration.

marcio-ao commented 6 years ago

@thinkyhead: This would require a little more memory, but could we have the highlight routine store the index of the highlighted item, and when the encoder is clicked, that stored selection would be activated, regardless of what the actual position of the encoder? This would ensure that nothing got selected that hadn't actually been highlighted.

thinkyhead commented 6 years ago

…store the index of the highlighted item…

I'll investigate some methods and come up with a patch asap.

I'm holding 1.1.9 just a few more days to shake out the last round of little issues like this and also to get some feedback on the (somewhat flawed) CR-10S power-loss recovery method after I adapt it to the current Marlin branches. I won't get into that topic here!

marcio-ao commented 6 years ago

some feedback on the (somewhat flawed) CR-10S power-loss recovery method

For what it is worth, I would much rather see Prussa's recover-on-print-crash ported over than the CR-10S's broken power-loss recovery method.

thinkyhead commented 6 years ago

It's on the TODO list for Marlin 2.0 / 2.1.

marcio-ao commented 6 years ago

Closing this ticket since things look much better in 1.1.9 and we are no longer seeing problems with scrolling.

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.