Fix for the PR #5088 (Handle M108 in M1 also with ULTIPANEL)

[ghost]

Fix for the PR #5088 (Handle M108 in M1 also with ULTIPANEL)

It fix a compilation error, and suppress a compilation warning.

When both EMERGENCY_PARSER and REPRAP_DISCOUNT_SMART_CONTROLLER are enabled, compilation error occurs.

error message

\AppData\Local\Temp\arduino_build_695096\sketch\Marlin_main.cpp: In function 'void gcode_M0_M1()':

Marlin_main.cpp:4435: error: 'wait_for_user' was not declared in this scope

       wait_for_user = true;

       ^

exit status 1
'wait_for_user' was not declared in this scope

And when EMERGENCY_PARSER is enabled and no LCD is enabled, compilation warning occurs.

warning message

\AppData\Local\Temp\arduino_build_695096\sketch\Marlin_main.cpp: In function 'void gcode_M0_M1()':

\AppData\Local\Temp\arduino_build_695096\sketch\Marlin_main.cpp:4473:1: warning: label 'ExitM1' defined but not used [-Wunused-label]

 ExitM1:

 ^

[thinkyhead]

Actually #5089 will supersede this. Do you mind giving #5089 a test to make sure LCD clicks still work without issue?

[ghost]

Oh, I did't know that.
And I've tested #5089 with real thing of REPRAP_DISCOUNT_SMART_CONTROLLER just now,
compilation error and warning doesn't occurs.

But, click-button of LCD not respond.
To be precise, it respond rarely, but it dosn't turn into menu, only LCD flickering once.

[thinkyhead]

Darn, I had hoped #5089 would do much better. I wish I had equipment to test with. Can you spot any obvious bugs or typos in the PR?

[thinkyhead]

I've made a small adjustment to #5089 but I don't expect any difference, unless ignore_click is somehow being set.

[ghost]

I've tested updated #5089, but problem isn't solved unfortunately.
click-button respond often(but not all-time), but still dosn't turn into menu, only LCD flickering once.

[thinkyhead]

@esenapaj It sounds like the click is entering the menu, but then it's immediately exiting the menu because it's not being ignored on the next loop. So it needs a universal debouncer.

[ghost]

Sorry, I tried to fix the PR #5089, but all failed.

[thinkyhead]

Sorry, I tried to fix the PR #5089, but all failed

Hmm. Never mind #5089 for now. It needs a re-think. Though it's not a bad idea, it's still flawed right now. I'll keep working on it, especially when I can get hold of an LCD controller. (I'm still traveling and don't have access.)

My idea was to make a single central place to manage LCD clicks, including handling de-bounce and wait-for-user conditions. But the lcd_clicked variable actually needs to be set in lcd_update.

[thinkyhead]

We don't need continuous-press for anything, so it seems fine to do it this way:

• Get lcd_clicked from the hardware buttons upon entry to the lcd_update function.
• If wait_for_unclick is set then ignore the button, check for a release.
• If wait_for_user is set then ignore the button, clear the wait_for_user flag.
• If the button goes through, then keep lcd_clicked set, but prepare to debounce…

I note that it can't just debounce right away. It has to wait until some function tries to read (and presumably use) the state of lcd_clicked and only then apply the debounce.

The good thing about this approach would be that if you pressed the button, lcd_clicked would get set and remain set until some handler comes along to use it (or until the button is released). I believe at present that the button flags are updated continually, but then the button is only tested and utilized every 10x through the loop. So, if it's pressed during those 10 loops, the button bits remain set for the whole time, and they only get reset after being read for use.

I may be slightly off in my assessment of that…

[thinkyhead]

Ok, so I updated the PR again. You can take a look to see if the new approach makes sense. It might totally fail too. But I think it's better than before. It will probably work in some partial way.

I hope it works, because it's more centralized and simplifies things.