-
-
Notifications
You must be signed in to change notification settings - Fork 954
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
Applications selection at build time #1894
Conversation
Build size and comparison to main:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some nitpicks. Not a proper review of the hard bits
Nice work! One more what's next idea that is in my mind: dependency relations to settings and controllers of apps. In a perfectly modular system, each app would bundle its settings and background tasks/controllers that get registered to the system.
It's true, but I wonder how much of the compile-time magic must be changed to runtime for that. But let's focus on that later. Now I have to find some time to polish the calculator and test this PR, because if optional apps get merged, it will be a huge step! Thank you for spending the time doing it 🥳 |
This seems huge, can it be used/extended for watchfaces too? |
Thanks for your feedback! I had this idea in my head for quite some time but... templates are not easy... I honestly can't tell how many hours of CPPCON video I watched before I could write such code :D
That's probably one of the next steps ! I think we could reduce the coupling between apps and settings/systemtask by having the app "register" to some system events and also by providing them an object that allows them to write their own settings in the file.
The compile time magic probably won't apply for dynamic apps, but having all the apps behave the same way (like constructing them using the same
Thank, and thank you for your contributions!
I already have a couple of ideas to used similar implementation for the watchfaces, yes. Watchfaces are probably the apps that need the most flash space! |
The CI check that fails is caused by InfiniSim that builds in C++17 while we now need C++20. I created a PR to fix this. |
A list of "user applications" is built at compile time. It contains all the info needed to create the application at runtime (ptr to a create() function) and to display the app in the application menu. All applications declare a TypeTrait with these information. When a new app must be loaded, DisplayApp first check if this app is a System app (in which case it creates it like it did before). If it's not a System app, it looks for the app in the list of User applications and creates it if it found it. Those changes allow to more easily add new app and to select which app must be built into the firmware. Switch to C++20 (and fix a few issues in SpiMaster.cpp and Watchdog.cpp.
Fix DisplayAppRecovery so it builds with -std=c++20.
Update documentation about the apps (Apps.md) : fix obsolete information, add doc about user/system apps and update the part about the implementation of a new app.
5641da9
to
67fa213
Compare
Why ?
InfiniTime is a monolithic firmware : everything (core OS, UI lib, BLE stack, apps) is built into a single firmware image. The current implementation does not allow to easily add new applications and select which apps must be built into the firmware.
This PR tries to improve those points.
Select which apps must be built into the firmware:
You can easily select the applications that will be built into the firmware by adding/removing app from UserAppTypes:
using UserAppTypes = TypeList<Apps::Alarm, Apps::HeartRate, Apps::Paint, Apps::Metronome, Apps::Music, Apps::Navigation, Apps::Paddle, Apps::Steps, Apps::StopWatch, Apps::Timer, Apps::Twos >; }
All the apps that are passed as template parameters to UserAppTypes will be built into the firmware. Moreover, the order in this list represents the order of the apps in the application menu.
The menu is automatically sized (number of pages and number of apps per page) to fit all the apps:
Add a new app
As always, start by implementing your app in a class that derives from Pinetime::Applications::Screens::Screen.
Add the .CPP file in the list of source files in the CMake file of the project.
Then, add a new enum value in Pinetime::Application::Apps
Additionally, declare a new AppTraits corresponding to your app. Here is an example from the Metronome app: You simply need to define the name, an icon, and a method
Create()
that creates the application:Finally, add the enum of your application in the template parameters of UserAppTypes
How ?
This is made possible by a bit of template meta programming magic in UserApps.h. Basically, the function
CreateAppDescriptions()
creates astd::array
of AppDescription at build time according to the enum values listed in UserAppTypes. It means that this array is created at build time and then stored in flash memory. It doesn't cost anything at runtime.Applications are now either "System applications" or "User applications". System applications are apps that are expected to be always built in the firmware (notifications, clock, settings,...). InfiniTime does not work properly if those apps are not present. User applications are applications that are optionally built into the firmware.
App creation
When a new app must be loaded, DisplayApp first checks if the app is a System app, in which case it simply creates it.
If it's not a System app, it looks into the list of User applications and creates it if it's found:
App menu
The App menu, ApplicationList now receives the list of apps as parameters. This list is generated by DisplayApp from the array of AppDescription generated at build time. It builds the menu pages according to this list of Apps.
Results
Developers can now add their application more easily than before, and no data is duplicated.
Developers and fork-maintainers can also select the apps they want to build in their firmware by editing a single line of code. This allows for easier customization of the firmware.
Firmware image sizes:
What's next ?