InfiniTimeOrg / InfiniTime

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

Hardcoded apps #1571

Closed Riksu9000 closed 8 months ago

Riksu9000 commented 1 year ago

Verification

Introduce the issue

There's an entry for every single screen in DisplayApp. To add an app, the app must be added in the app enum and the switch statement in LoadScreen must always be updated. This is too rigid, if our goal is to support third party apps, be it selectable at compile time, or dynamically.

Preferred solution

Removing the Apps enum.

Version

No response

LinuxinaBit commented 1 year ago

"At compile time" is something I hear a lot when referring to 3rd party apps, and I think we need to remember that most people don't want to recompile the OS every time an app is installed... Dynamically is probably best, but I'm not 100% sure exactly what that would entail.

Avamander commented 1 year ago

@RageGamerBoi

I think we need to remember that most people don't want to recompile the OS every time an app is installed...

I'm sure people are well-aware... It's just an inevitability at this point in time.

minacode commented 1 year ago

Is this issue targeting some kind of AppController class? I guess for compile-time we could just import everything once in there and make everything else that handles apps use that controller. For dynamically loaded apps we would then reimplement the class to search the file system and load the apps from there.

If there is any interest in (me) continuing #1408 from your side, that idea would fit there quite well, I think 😊

brainrom commented 1 year ago

My idea:

  1. Split screens to different apps with different CMakeLists
  2. Write code generator, which "register" all apps to a map (name and pointer to construction method)
  3. Store apps names as list on SPI flash and dynamically generate menu. This also allows to hide or rearrange apps without recompile.

According to recompilation for new apps installation, this isn't so bad. I have some developments, which allows to compile InfiniTime on Android smartphone with fancy UI. With ccache, it can be fast.

minacode commented 1 year ago

Splitting in apps would be nice.

Regarding the rest, do you have a good understanding how this this would work with FreeRTOS on the watch? There is some discussion about position independent code. Do you mean that?

brainrom commented 1 year ago

Position independent code allows to install binary apps inside already built firmware. This is the best solution, but at this point, this is too complicated. In my proposed solution, InfiniTime stays single binary, just compiled differently and allows slight modification of main menu without recompilation. Combining with "compilation on smartphone", it's almost enough.

minacode commented 1 year ago

Ok, I get it now. So your code generator would more or less generate something like an AppController. But then you add the extra capability of rearranging the menu via a config file.

I would say we do the following to work in little steps:

  1. Implement the Controller with the app code base as is and verify that it works.
  2. Maybe add the config file, if necessary.
  3. Change the directory structure and CMake.
  4. Future, like position independent code.

What do you think about this?

Riksu9000 commented 1 year ago

This issue is essentially about removing the Apps enum. As long as we have the enum, we can't load any app that's not specifically handled by the firmware. A code generator will not fix the core issue.

Riksu9000 commented 1 year ago

We need to figure out what to do about returnAppStack. Maybe we could store constructor references, but all the constructors would need to have the same parameters. (I thought you could do this but I might be making stuff up)

Do we pass the apps just a DisplayApp reference or something more restricted, and the apps access it to get all their information? Any other ideas?

minacode commented 1 year ago

We need to figure out what to do about returnAppStack. Maybe we could store constructor references, but all the constructors would need to have the same parameters.

The thing is, we need some sort of value for each app, regardless of if we have the enum or not. Besides the stack there is also the app menu that needs to communicate which app the user chose.

One idea I had was (take this as a sketch ):

class App : Screen {
  virtual string GetSymbol();
  virtual T GetId();
}

Having a reference as the id instead is a clever idea.

Do we pass the apps just a DisplayApp reference or something more restricted, and the apps access it to get all their information? Any other ideas?

This would be a facade and sounds like a good idea to provide a stable app-API to the developers.

I don't know about the internals of constructor references, but I think your idea sounds good and is worth trying.

minacode commented 1 year ago

I tried around a little bit and encountered a problem: how do we handle non-app screens? Does it make sense to dynamically register every possible screen to a screen manager? I don't think so. From my view there are:

  1. "core" screens: the app menu, quick settings, notifications, firmware update ...
  2. setting screens
  3. app screens

For the core screens a hardcoded enum seems fine to me (at least for now).

Settings are not as easy, because there are also app-related settings, but that is another issue. (I think we need separate setting files per app in the long run).

App screens should be the ones that are handled as dynamically as possible.

Update: Here is a quick idea that might be too hacky: we keep the enum (of uint8_t), rename it Screens, remove all the values for real apps from it and add the last value to be Screens::App. We can then use values >= Screens::App that are calculated by Screens::App + <some dynamic app id>. To load such an app screen, we query an AppController.

JF002 commented 8 months ago

A solution for this issue was implemented in #1894 .