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

Watch face selection using CMake #1934

Merged
merged 4 commits into from Jan 6, 2024
Merged

Watch face selection using CMake #1934

merged 4 commits into from Jan 6, 2024

Conversation

JF002
Copy link
Collaborator

@JF002 JF002 commented Dec 24, 2023

The list of watch face to build into the firmware is now set by CMake (-DENABLE_WATCHFACES).

Fix SettingWatchFace : convert to index to/from WatchFace when needed.

@JF002 JF002 added this to the 1.14.0 milestone Dec 24, 2023
Copy link

github-actions bot commented Dec 24, 2023

Build size and comparison to main:

Section Size Difference
text 369768B 208B
data 940B 0B
bss 63516B 0B

The list of watch face to build into the firmware is now set by CMake (-DENABLE_WATCHFACES).

Fix SettingWatchFace : convert to index to/from WatchFace when needed.
Update Apps.md to mention the selection of watchfaces using Cmake.
@vkareh
Copy link
Contributor

vkareh commented Jan 5, 2024

This works well for me (I rebased this locally against main). Disabling some of the faces I'm not personally interested in shaved off 23K from my image file as compared to main. That's a huge deal :)

Copy link
Contributor

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

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

watchface to watch face, some wording and a small cmake readability improvement

doc/code/Apps.md Outdated Show resolved Hide resolved
doc/code/Apps.md Outdated Show resolved Hide resolved
doc/code/Apps.md Outdated Show resolved Hide resolved
doc/code/Apps.md Outdated Show resolved Hide resolved
doc/code/Apps.md Outdated Show resolved Hide resolved
doc/code/Apps.md Outdated Show resolved Hide resolved
doc/code/Apps.md Outdated Show resolved Hide resolved
doc/code/Apps.md Outdated Show resolved Hide resolved
doc/code/Apps.md Outdated Show resolved Hide resolved
src/displayapp/apps/CMakeLists.txt Outdated Show resolved Hide resolved
Improve wording and replace "watchface" by "watch face" in Apps.md.
Improve CMake readability regarding watch face selection

Co-authored-by: Reinhold Gschweicher <pyro4hell@gmail.com>
@JF002
Copy link
Collaborator Author

JF002 commented Jan 6, 2024

@vkareh Thanks for the feedback! That was exactly the goal of those changes :+1

@NeroBurner Thanks for the review, I applied all your suggestions !

Copy link
Contributor

@NeroBurner NeroBurner left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Member

@FintasticMan FintasticMan left a comment

Choose a reason for hiding this comment

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

Almost missed this!

doc/code/Apps.md Outdated Show resolved Hide resolved
Documentation : watch faces are not system apps anymore.

Co-authored-by: FintasticMan <finlay.neon.kid@gmail.com>
@JF002 JF002 merged commit 6505336 into main Jan 6, 2024
7 checks passed
@JF002 JF002 deleted the cmake-watchface-selection branch January 6, 2024 13:44
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

4 participants