[1.1.x] Fix LCD button / newbutton issue#13157
Merged
thinkyhead merged 1 commit intoMarlinFirmware:bugfix-1.1.xfrom Feb 14, 2019
Merged
[1.1.x] Fix LCD button / newbutton issue#13157thinkyhead merged 1 commit intoMarlinFirmware:bugfix-1.1.xfrom
thinkyhead merged 1 commit intoMarlinFirmware:bugfix-1.1.xfrom
Conversation
1) Minor: buttons is a volatile variable. button |= slow_buttons should not be done if newbuttons |= ... can be done 2) Major: #if ENABLED(ADC_KEYPAD) ... buttons = 0 is done after buttons = newbuttons The values from a rotary encoder (which I installed) are erased by this line, and newbuttons would have been 0 anyway if there was no button. TL;DR: Don't undo reading the rotary encoder before reading the ADC keys
wmandra
added a commit
to wmandra/Marlin
that referenced
this pull request
Feb 13, 2019
Logic errors in ultralcd.cpp 1) Minor: buttons is a volatile variable. button |= slow_buttons should not be done if newbuttons |= ... can be done 2) Major: #if ENABLED(ADC_KEYPAD) ... buttons = 0 is done after buttons = newbuttons The values from a rotary encoder (which I installed) are erased by this line, and newbuttons would have been 0 anyway if there was no button. TL;DR: Don't undo reading the rotary encoder before reading the ADC keys
Member
|
Marlin 1.1.x is end-of-life and won't get any new releases, so you should focus on Marlin |
thinkyhead
changed the title
thinkyhead
changed the title
thinkyhead
pushed a commit
that referenced
this pull request
Sep 2, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Logic errors in ultralcd.cpp
Minor:
buttons is a volatile variable. button |= slow_buttons should not be done
if newbuttons |= ... can be done
Major:
#if ENABLED(ADC_KEYPAD) ... buttons = 0 is done after buttons = newbuttons
The values from a rotary encoder (which I installed) are erased by this
line, and newbuttons would have been 0 anyway if there was no button.
TL;DR: Don't undo reading the rotary encoder before reading the ADC keys
Benefits // Related Issues
I created a display having a rotary encoder and back/home buttons by using an ADC_KEY. Being in a deep menu, these extra keys are helpful, while also giving the benefit of having a rotary encoder instead of emulating one using the up/down keys. The encoder would not work even though the pins were read correctly.