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.

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

WIP: Initial menu implementation with rotary encoder #383

Closed
wants to merge 15 commits into from

Conversation

FabianSperrle
Copy link
Contributor

define menu structure for setting brew temp and steam temp (other entries for times etc. could easily be added)
add rotary encoder library to read encoder state

TODO:

  • no support for upright displays yet
  • multilanguage support started but not yet finished
  • Pins need to be defined in userConfig.h at the moment
  • I don't like that all the code is in the "main" file, making it even longer and more cluttered. However, I did not manage to move the display rendering code to displays.h yet as I always faced issues with variables not being defined.

I'm curious to hear your feedback and ideas on how to proceed with this, if you are interested.

@fiendie
Copy link
Member

fiendie commented Oct 16, 2023

You renamed userConfig_sample.h to unserconfig_sample.h, that's why the build is failing :)

@fiendie
Copy link
Member

fiendie commented Dec 9, 2023

I finally had time to test it further.
The menu itself works great.

The three issues I noticed during testing:

  • displayMenu() uses the font size that is currently displayed on screen so it's too large when the cold start screen is displayed for example
  • The ESP crashes when the menu is open and I try to access the website
  • I still get the occasional downward spike in temperature

I took the liberty of fixing the first issue and some code style issues along with it :)
If you want to have a look, I pushed it to https://github.com/fiendie/clevercoffee/tree/rotary
Maybe you can cherry-pick the commit and rebase/squash it into your changes so you retain full authorship for the changes. I just added the commit so it's clear what I actually changed.

@FabianSperrle
Copy link
Contributor Author

Continued in #419

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants