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

Improve MainMenu, add brightness control #25

Merged
merged 6 commits into from
Sep 6, 2024

Conversation

TokenRat
Copy link
Collaborator

@TokenRat TokenRat commented Sep 6, 2024

Improve MainMenu, add brightness control

Somehow it is not yet correctly persisted. I think I am loading globals incorrectly in main.cpp and that makes it reset this to the default value.

Somehow it is not yet correctly persisted. I think I am loading globals incorrectly in main.cpp and that makes it reset this to the default value.
@TokenRat
Copy link
Collaborator Author

TokenRat commented Sep 6, 2024

Based on #24 , merge this one first.

Copy link
Collaborator

@ngandrass ngandrass left a comment

Choose a reason for hiding this comment

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

There is a 15-character limit for keys in Preferences / NVS. This most likely causes problems.

Namespace and key names are character strings and are limited to a maximum of 15 characters.

See: https://docs.espressif.com/projects/arduino-esp32/en/latest/tutorials/preferences.html#preferences-attributes

Base automatically changed from fix/tweak_values to main September 6, 2024 10:34
Copy link
Collaborator

@ngandrass ngandrass left a comment

Choose a reason for hiding this comment

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

I pushed a potential fix but cannot test it right now.

@TokenRat Please test, confirm its working properly, and merge if you like 👍

@ngandrass
Copy link
Collaborator

Ok, there were more problems. A second FSMGlobals instance was created and the brightness was tried to be set before the values were read from the NVS. Now it should work and I did a super quick test on hardware.

Please test yourself and merge afterwards ❤️

@TokenRat TokenRat merged commit 6d18129 into main Sep 6, 2024
1 check passed
@TokenRat TokenRat deleted the feature/brightness_control branch September 6, 2024 14:21
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