GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — User Applications selection using CMake by JF002 · Pull Request #1928 · InfiniTimeOrg/InfiniTime Conversation
Collaborator
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.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
|
There was an error while loading. Please reload this page.
|
Build size and comparison to main:
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Thanks! I've just updated the doc to add instructions related to CMake. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| 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) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I'll have a look at this!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Done in c200816 and integrated in InfiniSim here : InfiniTimeOrg/InfiniSim#134
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| if(${enabled}) | ||
| list(APPEND ${list} ${type}) | ||
| endif () | ||
| #return(PROPAGATE ${list}) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Not enabled, but comment in PR sais it is
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
The version of CMake in the docker container is too old for this. See the second commit in this PR.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
I don't know much about CMake, but could we put all optional apps in a separate directory and then autogenerate the CMake options? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 😄 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
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 :)
Good point! I'll try to find a solution for this! EDIT : Applied @FintasticMan 's suggestion in 00bb624 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
|
||
| message("PROJECT_GIT_COMMIT_HASH_SUCCESS? " ${PROJECT_GIT_COMMIT_HASH_SUCCESS}) | ||
|
|
||
| if(DEFINED ENABLE_USERAPPS) |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Good point, done!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@NeroBurner I'll apply the same change to the watchface selection when you confirm this is ok for you :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Handling such dependencies would be a next step. Apps also come with custom BLE interfaces (e.g. music). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
small typos, afterwards good to go from my side 🎉
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
| 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). |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
use one line per sentence, makes the diffs in future easier, and "user applications"
| 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). |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| 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) No newline at end of file |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
add newline to end of file
| 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) | |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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 <[email protected]>
Fix typos in Apps.md and add new line in src/displayapp/apps/CMakeLists.txt
Fix include path since last rebase.
0fe4ed3 to
f69983e
Compare
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 #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.
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.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.