User Applications selection using CMake #1928

JF002 · 2023-12-17T15:35:03Z

Use CMake's configure_file() functionality to generate the list of User Applications.

All the apps included in current versions of InfiniTime are enabled by default, but this can now be overridden by setting variables ENABLE_APP_XXX to True or False.

CMake CMP0140 is set to NEW to enable the return PROPAGATE functionality.

github-actions · 2023-12-17T15:38:08Z

Build size and comparison to main:

Section	Size	Difference
text	368984B	-16B
data	940B	0B
bss	63516B	0B

FintasticMan

This looks good! It allows a lot more freedom for building with whatever apps you want. I would say we should add this to the Apps.md documentation before merging, as you now need to register new apps in more places.

JF002 · 2023-12-17T16:18:51Z

This looks good! It allows a lot more freedom for building with whatever apps you want. I would say we should add this to the Apps.md documentation before merging, as you now need to register new apps in more places.

Thanks! I've just updated the doc to add instructions related to CMake.

FintasticMan · 2023-12-17T16:59:01Z

It seems like InfiniSim will also need to include these same option declarations. @NeroBurner do you know if it would be possible to have InfiniSim get the options from InfiniTime?

NeroBurner · 2023-12-17T21:15:00Z

CMakeLists.txt

@@ -35,6 +34,19 @@ endif()
 set(TARGET_DEVICE "PINETIME" CACHE STRING "Target device")
 set_property(CACHE TARGET_DEVICE PROPERTY STRINGS PINETIME MOY-TFK5 MOY-TIN5 MOY-TON5 MOY-UNK)

+option(ENABLE_APP_STOPWATCH "Enable the Stopwatch application" True)


We can move those options and the Apps.h.in file into its own directory, create a CMake header only library target and link that to everywhere we need it. Because it is in its own folder I can easily include it in InfiniSim afterwards. Like we do for the fonts and images

I'll have a look at this!

Done in c200816 and integrated in InfiniSim here : InfiniTimeOrg/InfiniSim#134

NeroBurner · 2023-12-17T21:17:32Z

CMakeLists.txt

+  if(${enabled})
+    list(APPEND ${list} ${type})
+  endif ()
+  #return(PROPAGATE ${list})


Not enabled, but comment in PR sais it is

The version of CMake in the docker container is too old for this. See the second commit in this PR.

minacode · 2023-12-18T19:22:25Z

I don't know much about CMake, but could we put all optional apps in a separate directory and then autogenerate the CMake options?

.../displayapp/
    apps/
        music/
            music.cpp
            music.h
        twos/
            twos.cpp
            twos.h
        ...

FintasticMan · 2023-12-18T19:33:10Z

I would imagine that that would be possible, and it would improve the dev experience quite a lot, not having to remember every single place to add a new app 😄

FintasticMan · 2023-12-19T11:12:57Z

One thing I've just thought of is that this doesn't allow choosing the order the apps appear in the app list. I think that that is quite an important feature, so we should try to figure out a way to get that to work.

One simple way would just be to have a string CMake parameter that gets placed in UserAppTypes.

JF002 · 2023-12-19T14:47:48Z

@minacode

I don't know much about CMake, but could we put all optional apps in a separate directory and then autogenerate the CMake options?

It might be possible, yes. Actually, I would like InfiniTime to support User Applications coming from external repositories. This would allow maintainers of those user apps to maintain their apps the way they want, and to reduce the load on InfiniTime maintainers. I think we can do this in another future PR though :)

@FintasticMan

One thing I've just thought of is that this doesn't allow choosing the order the apps appear in the app list. I think that that is quite an important feature, so we should try to figure out a way to get that to work.

Good point! I'll try to find a solution for this! EDIT : Applied @FintasticMan 's suggestion in 00bb624

NeroBurner · 2023-12-19T18:45:27Z

CMakeLists.txt

@@ -51,6 +45,12 @@ string(STRIP "${PROJECT_GIT_COMMIT_HASH}" PROJECT_GIT_COMMIT_HASH)

 message("PROJECT_GIT_COMMIT_HASH_SUCCESS? " ${PROJECT_GIT_COMMIT_HASH_SUCCESS})

+if(DEFINED ENABLE_USERAPPS)


I'd move this variable definition into src/displayapp/apps/CMakeLists.txt. then we'll have one place to update the default apps list and won't need to change the default list in InfiniSim as well.

Otherwise I'd need to update InfiniSim everytime the app list is changed to match InfiniTimes default

Good point, done!

JF002 · 2023-12-21T19:54:39Z

@NeroBurner I'll apply the same change to the watchface selection when you confirm this is ok for you :)

mark9064 · 2023-12-22T18:38:25Z

What about apps that introduce settings pages specific to them? We don't actually have any right now (arguably steps), but some are proposed e.g background heart rate settings

minacode · 2023-12-22T21:52:26Z

Handling such dependencies would be a next step. Apps also come with custom BLE interfaces (e.g. music).

JF002 · 2023-12-23T18:49:15Z

@mark9064 That's a good question! And I agree with @minacode, we can figure that out in a next step. In this current state, the worse that can happen is that settings will show a setting that no userapp will actually use, but the whole firmware will still be functional.

NeroBurner

small typos, afterwards good to go from my side 🎉

doc/code/Apps.md

NeroBurner · 2023-12-23T19:13:28Z

doc/code/Apps.md

@@ -159,6 +159,12 @@ If your application is a **user** application, you don't need to add anything in
 everything will be automatically generated for you.
 The user application will also be automatically be added to the app launcher menu.

+Since the list of **user** application is generated by CMake, you need to add the variable `ENABLE_USERAPPS` to the command line of CMake. This variable must be set with a string composed of an ordered list of the **user** applications that must be built into the firmware. The items of the list are fields from the enumeration `Apps`. Ex : build the firmware with 3 user application : Alarm, Timer and MyApp (the application will be listed in this specific order in the application menu).


use one line per sentence, makes the diffs in future easier, and "user applications"

Suggested change

Since the list of **user** application is generated by CMake, you need to add the variable `ENABLE_USERAPPS` to the command line of CMake. This variable must be set with a string composed of an ordered list of the **user** applications that must be built into the firmware. The items of the list are fields from the enumeration `Apps`. Ex : build the firmware with 3 user application : Alarm, Timer and MyApp (the application will be listed in this specific order in the application menu).

Since the list of **user** applications is generated by CMake, you need to add the variable `ENABLE_USERAPPS` to the command line of CMake.

This variable must be set with a string composed of an ordered list of the **user** applications that must be built into the firmware.

The items of the list are fields from the enumeration `Apps`.

Ex : build the firmware with 3 user applications : Alarm, Timer and MyApp (the applications will be listed in this specific order in the application menu).

NeroBurner · 2023-12-23T19:15:13Z

src/displayapp/apps/CMakeLists.txt

+target_include_directories(infinitime_apps INTERFACE "${CMAKE_CURRENT_BINARY_DIR}/")
+
+# Generate the list of user apps to be compiled into the firmware
+configure_file(${CMAKE_CURRENT_SOURCE_DIR}/Apps.h.in ${CMAKE_CURRENT_BINARY_DIR}/Apps.h)


add newline to end of file

Suggested change

configure_file(${CMAKE_CURRENT_SOURCE_DIR}/Apps.h.in ${CMAKE_CURRENT_BINARY_DIR}/Apps.h)

configure_file(${CMAKE_CURRENT_SOURCE_DIR}/Apps.h.in ${CMAKE_CURRENT_BINARY_DIR}/Apps.h)

Use CMake's configure_file() functionality to generate the list of User Applications. All the apps included in current versions of InfiniTime are enabled by default, but this can now be overridden by setting variables ENABLE_APP_XXX to True or False. CMake CMP0140 is set to NEW to enable the return PROPAGATE functionality.

Revert changes that need "return PROPAGATE" since this is not available in our Docker build (it needs CMake 3.25 and we have 3.22).

Update documentation about building a new application and add instructions to add the app in CMake files.

Replace the options that allowed to select the user apps independently by a single string variable that contains the ordered list of apps to build.

Move displayapp/Apps.h into a header only library (to make the integration easier in InfiniSim.

Move ENABLE_USERAPPS and USERAPP_TYPES from the root CMake file to src/displayapp/apps/CMakeLists.txt so we do not need to repeat it in InfiniSim

Co-authored-by: NeroBurner <pyro4hell@gmail.com>

Fix typos in Apps.md and add new line in src/displayapp/apps/CMakeLists.txt

Fix include path since last rebase.

Restore the default list of apps to compile. The ordering was changed in the changeset to make the app-list configurable through a CMake-variable in #1928 In the process have one app per line to create the default app list in CMake. This makes git diffs easer and more readable.

Restore the default list of apps to compile. The ordering was changed in the changeset to make the app-list configurable through a CMake-variable in InfiniTimeOrg#1928 In the process have one app per line to create the default app list in CMake. This makes git diffs easer and more readable.

FintasticMan requested changes Dec 17, 2023

View reviewed changes

FintasticMan approved these changes Dec 17, 2023

View reviewed changes

NeroBurner reviewed Dec 17, 2023

View reviewed changes

FintasticMan approved these changes Dec 19, 2023

View reviewed changes

NeroBurner reviewed Dec 19, 2023

View reviewed changes

NeroBurner approved these changes Dec 23, 2023

View reviewed changes

JF002 and others added 9 commits December 23, 2023 21:15

CMake User Applications selection

96727c5

Revert changes that need "return PROPAGATE" since this is not available in our Docker build (it needs CMake 3.25 and we have 3.22).

CMake User Applications selection

20c9aad

Update documentation about building a new application and add instructions to add the app in CMake files.

Watch face selection at build time

7ed65d3

Replace the options that allowed to select the user apps independently by a single string variable that contains the ordered list of apps to build.

Watch face selection with CMake

a4577f3

Move displayapp/Apps.h into a header only library (to make the integration easier in InfiniSim.

CMake user application selection

009c1d2

Move ENABLE_USERAPPS and USERAPP_TYPES from the root CMake file to src/displayapp/apps/CMakeLists.txt so we do not need to repeat it in InfiniSim

Fix trailing space in doc/code/Apps.md

996c045

Co-authored-by: NeroBurner <pyro4hell@gmail.com>

User applications selection using CMake

5aae9fe

Fix typos in Apps.md and add new line in src/displayapp/apps/CMakeLists.txt

User applications selection using CMake

f69983e

Fix include path since last rebase.

JF002 force-pushed the cmake-userapps-selection branch from 0fe4ed3 to f69983e Compare December 23, 2023 20:24

JF002 added this to the 1.14.0 milestone Dec 23, 2023

JF002 merged commit fc5424c into main Dec 23, 2023
6 of 7 checks passed

JF002 deleted the cmake-userapps-selection branch December 23, 2023 20:29

NeroBurner mentioned this pull request Jan 6, 2024

apps: restore default apps ordering in CMake #1952

Merged

JF002 mentioned this pull request Mar 19, 2024

[WIP/RFC] Remove watchface enum #2040

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User Applications selection using CMake #1928

User Applications selection using CMake #1928

JF002 commented Dec 17, 2023

github-actions bot commented Dec 17, 2023 •

edited

Loading

FintasticMan left a comment

JF002 commented Dec 17, 2023

FintasticMan commented Dec 17, 2023

NeroBurner Dec 17, 2023

JF002 Dec 19, 2023

JF002 Dec 19, 2023

NeroBurner Dec 17, 2023

FintasticMan Dec 17, 2023

minacode commented Dec 18, 2023 •

edited

Loading

FintasticMan commented Dec 18, 2023

FintasticMan commented Dec 19, 2023

JF002 commented Dec 19, 2023 •

edited

Loading

NeroBurner Dec 19, 2023

JF002 Dec 21, 2023

JF002 commented Dec 21, 2023

mark9064 commented Dec 22, 2023

minacode commented Dec 22, 2023

JF002 commented Dec 23, 2023

NeroBurner left a comment

NeroBurner Dec 23, 2023

NeroBurner Dec 23, 2023

		@@ -51,6 +45,12 @@ string(STRIP "${PROJECT_GIT_COMMIT_HASH}" PROJECT_GIT_COMMIT_HASH)

		message("PROJECT_GIT_COMMIT_HASH_SUCCESS? " ${PROJECT_GIT_COMMIT_HASH_SUCCESS})

		if(DEFINED ENABLE_USERAPPS)

	configure_file(${CMAKE_CURRENT_SOURCE_DIR}/Apps.h.in ${CMAKE_CURRENT_BINARY_DIR}/Apps.h)
	configure_file(${CMAKE_CURRENT_SOURCE_DIR}/Apps.h.in ${CMAKE_CURRENT_BINARY_DIR}/Apps.h)

User Applications selection using CMake #1928

User Applications selection using CMake #1928

Conversation

JF002 commented Dec 17, 2023

github-actions bot commented Dec 17, 2023 • edited Loading

FintasticMan left a comment

Choose a reason for hiding this comment

JF002 commented Dec 17, 2023

FintasticMan commented Dec 17, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minacode commented Dec 18, 2023 • edited Loading

FintasticMan commented Dec 18, 2023

FintasticMan commented Dec 19, 2023

JF002 commented Dec 19, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JF002 commented Dec 21, 2023

mark9064 commented Dec 22, 2023

minacode commented Dec 22, 2023

JF002 commented Dec 23, 2023

NeroBurner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 17, 2023 •

edited

Loading

minacode commented Dec 18, 2023 •

edited

Loading

JF002 commented Dec 19, 2023 •

edited

Loading