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.

Sign up for GitHub

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

Jump to bottom

Reduce size of menu code #1620

Merged
merged 6 commits into from
Mar 19, 2015
Merged

Reduce size of menu code #1620

merged 6 commits into from
Mar 19, 2015

Conversation

thinkyhead
Copy link
Member

  • Get rid of _selected functions, passing selected state instead
  • Reduces code size by 6 - 7K !

@thinkyhead
Reduce size of menu code
3405f85 
- Get rid of _selected functions, passing selected state instead
@thinkyhead thinkyhead mentioned this pull request Mar 16, 2015
Closed
@thinkyhead
Cleanup for menu optimization
7629c4f 
- Compare pr_char to space
- Fewer calls to lcd_strlen
@AnHardt
Copy link
Member

AnHardt commented Mar 16, 2015

@thinkyhead
In https://github.com/MarlinFirmware/Marlin/blob/Development/Marlin/dogm_lcd_implementation.h#L284
the value of the prechar is not used anymore since we use highlighting. You can save some more decisions by passing sel directly.

@thinkyhead
Copy link
Member Author

@AnHardt Good idea! I guess I stuck to that because it was the convention, so it will be good to get past that and make the code more properly expressive.

@boelle
Copy link
Contributor

boelle commented Mar 19, 2015

this one seems logic to merge... but is it to early?

@thinkyhead
Merge branch 'Development' into optimize_menu_code
a547dc2 
Get upstream changes
@thinkyhead
Pass sel to lcd_implementation_mark_as_selected
15d29ef 
- Pass selected state directly to lcd_implementation_mark_as_selected
- Rename sel function parameter
- Include a minor fix for SdBaseFile.h
@thinkyhead
Copy link
Member Author

With this last commit it should be ready. Waiting for Travis to pass it.

@thinkyhead
Include indicator for new drawmenu_generic
90ba61b 
In the new method we pass the character that should be used for
selected state, not the character to print always.
@thinkyhead
Small code tweaks re lcd.print
7279e4f
thinkyhead added a commit that referenced this pull request Mar 19, 2015
@thinkyhead
Merge pull request #1620 from thinkyhead/optimize_menu_code
0858fba 
Reduce size of menu code
@thinkyhead thinkyhead merged commit 0858fba into MarlinFirmware:Development Mar 19, 2015
@AnHardt
Copy link
Member

AnHardt commented Mar 20, 2015

@thinkyhead
There is a problem in 'ultralcd_implementation_hitachi_HD44780.h'.
While scroling down a menu all if fine until the line marker '>' reaches the ((bottom line) and (the end of the menu)).
When turning further the line marker vanishes.

May be the same as:
#1209 (comment)

@AnHardt
Copy link
Member

AnHardt commented Mar 20, 2015

@thinkyhead
'>' also not drawn for the 'edit' menu items.

@thinkyhead thinkyhead mentioned this pull request Mar 20, 2015
Merged
@thinkyhead
Copy link
Member Author

@AnHardt #1653 should fix the edit items. Not sure yet what's causing newly-scrolled items not to show as hilited.

@AnHardt
Copy link
Member

AnHardt commented Mar 20, 2015

@thinkyhead
Yes, #1653 fixed the edit items.

No, not newly-scrolled items.
It makes the impression as if the '>' if scrolling out of view at the end of the list when you try to go beyond last item.

@AnHardt AnHardt mentioned this pull request Mar 20, 2015
Merged
@AnHardt
Copy link
Member

AnHardt commented Mar 20, 2015

@thinkyhead
This was really hard to find. Cost me hours. But at least i have a much better insight in the menu-code now.

@thinkyhead
Copy link
Member Author

@AnHardt I see, so the menu was failing to scroll. Well, I'm excited you found the issue. Checking out the code now....

@thinkyhead thinkyhead deleted the optimize_menu_code branch March 25, 2015 06:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@thinkyhead @AnHardt @boelle