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

Refactor main menu functionality and state handling #205

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

kolemikko
Copy link

This PR is my proposal how to refactor/reimplement the main menu functionality and state handling.

The aim is to simplify the code structure, avoid duplicate code and allow better extensibility. But also, to make menu flows easier to follow and isolate state handling within certain menu rather than handling all possible states at handleButtonPress().

Main changes:

  • showMenu() and showFastMenu() are replaced by showMainMenu() and getMenuSelection() methods. In showMainMenu() we define the elements needed for the main menu and handle its flow based on the user selection. getMenuSelection() serves as a generic method for creating menus and can be easily used for creating new submenus etc. Button inputs and menu specific timeout value are handled inside getMenuSelection(). In case of main menu, at timeout we get out of the menu and handle flow after in showMainMenu().

  • handleButtonPress() isn't handling states anymore, for now we only wake up from menu button and go to main menu.

  • Going into apps sets APP_STATE, which also defines how the flow after the app goes. In showMainMenu() we go back to main menu from APP_STATE or to WATCHFACE_STATE if going back because of timeout or back button. Since APP_STATE defines the flow after the app is done, we don't have to call showMainMenu() every time we should go back to main menu from app.

  • Term menu used in global variables is changed to mainMenu to be more clear and not to confuse with other menus in the future.

  • State FW_UPDATE_STATE is removed since APP_STATE can be used as well.

  • Firmware update app shows example of submenu flow where button inputs inside a menu define what happens next, e.g. do we go forward within the app or go back to main menu.

  • RTC alarm wakeup handles only WATCHFACE_STATE since we never stay in the main menu because of timeout.

  • Global MENU_LENGTH is removed since menu length is menu specific and has to be given to getMenuSelection() as an argument.

This is fairly big change but I haven't noticed anything being broken, any feedback is welcome! :)

@kolemikko kolemikko changed the base branch from master to dev December 31, 2022 13:18
@DarkZeros
Copy link
Contributor

I have been experimenting with multiple ways to simplify menus, I have nothing nearly "done" but I think this is a step forward.
There are multiple ways to improve it for the future also to take into account that I have experimented with:

  • Generate a definition structure for menus (maybe using constexpr arrays nesting, or other ways that is fast), I have tried with JSON and works, but is slow.
  • Rather than reading input buttons, use interrupts to store button presses, that way, if a button is pressed while updating screen, the press will not be lost. The problem with interrupts is that double presses are possible...

Copy link
Contributor

@khenderick khenderick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works fine, and I think it's a step in the right direction.

@denics denics mentioned this pull request Apr 17, 2023
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.

None yet

3 participants