InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.64k stars 902 forks source link

User Applications selection using CMake #1928

Closed JF002 closed 6 months ago

JF002 commented 6 months ago

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[bot] commented 6 months ago
Build size and comparison to main: Section Size Difference
text 368984B -16B
data 940B 0B
bss 63516B 0B
JF002 commented 6 months ago

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 commented 6 months ago

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?

minacode commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

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 commented 6 months ago

@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

JF002 commented 6 months ago

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

mark9064 commented 6 months ago

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 commented 6 months ago

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

JF002 commented 6 months ago

@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.