Skip to content
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

Hardcoded apps #1571

Closed
1 task done
Riksu9000 opened this issue Jan 29, 2023 · 12 comments
Closed
1 task done

Hardcoded apps #1571

Riksu9000 opened this issue Jan 29, 2023 · 12 comments
Labels
help wanted Extra attention is needed
Milestone

Comments

@Riksu9000
Copy link
Contributor

Riksu9000 commented Jan 29, 2023

Verification

  • I searched for similar issues and found none was relevant.

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

@Riksu9000 Riksu9000 added the help wanted Extra attention is needed label Jan 30, 2023
@LinuxinaBit
Copy link

"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
Copy link
Collaborator

@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
Copy link
Contributor

minacode commented Feb 2, 2023

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
Copy link

brainrom commented Feb 3, 2023

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
Copy link
Contributor

minacode commented Feb 3, 2023

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
Copy link

brainrom commented Feb 4, 2023

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
Copy link
Contributor

minacode commented Feb 4, 2023

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 Riksu9000 mentioned this issue Mar 5, 2023
1 task
@Riksu9000
Copy link
Contributor Author

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
Copy link
Contributor Author

Riksu9000 commented Mar 14, 2023

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
Copy link
Contributor

minacode commented Mar 14, 2023

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
Copy link
Contributor

minacode commented Apr 2, 2023

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
Copy link
Collaborator

JF002 commented Dec 10, 2023

A solution for this issue was implemented in #1894 .

@JF002 JF002 closed this as completed Dec 10, 2023
@JF002 JF002 added this to the 1.14.0 milestone Dec 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

6 participants