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

PROS 4: Decouple LVGL and Make LVGL Template #168

Closed
HotelCalifornia opened this issue Sep 12, 2019 · 29 comments
Closed

PROS 4: Decouple LVGL and Make LVGL Template #168

HotelCalifornia opened this issue Sep 12, 2019 · 29 comments
Assignees
Labels
rfc This describes a feature, enhancement, or optimization in broad terms and should be discussed

Comments

@HotelCalifornia
Copy link
Contributor

Motivation

Currently, LVGL is quite tightly coupled with the kernel, which makes it somewhat of a nightmare to upgrade. Further, when compiling the kernel, we have to also compile all of LVGL which takes a not insignificant amount of time.

As such, I propose (as some others have proposed before me, namely @3038922 and @theol0403) that we separate LVGL out into its own template, and include it in the default_templates list akin to how OkapiLib is.

Benefits

As mentioned in the Motivation section, the main benefit will be to make LVGL easier to upgrade without hassle. A side benefit of this is that we will also be able to offer multiple, potentially incompatible versions of LVGL. For example, we will be able to provide both templates for both versions 5.3 and 6.0, and users will be able to choose which they prefer to use.
Also mentioned in the Motivation section, kernel compile time will be reduced, and we won't have to deal with seeing LVGL warnings show up in kernel builds (e.g. on Azure).

Technical issues

There are a number of technical concerns surrounding this proposal. I'll go over the ones I've thought about here.

  • The touch driver and flush function use functions from libv5rts. This ends up not being much of a problem, as we can simply forward declare the functions and types we use.
  • disp_daemon is a static task. Same deal here. We can either forward declare the relevant static task function(s) or just decide to expose them to users in pros/apix.h.
  • display_initialize should be called during low-level initialization. The only requirement for users is that all LVGL setup is completed before initialize or competition_initialize are called. As such, all we'd need to do is mark the function with __attribute__((constructor)).
  • What about internal kernel functions that use LVGL? To my knowledge the only place the kernel refers to LVGL (aside from LLEMU, which will be addressed next) is for the VDML port unplugging/mismatch warning. We might want to consider removing this warning entirely, as it duplicates vexOS functionality (that was not present when the system was initially implemented).
  • What about LLEMU? LLEMU may also be refactored out into this separate template. If we make it a default template, then new projects will still work. Advanced users who modify their default templates should know that removing this will break the default PROS kernel template code.
@HotelCalifornia HotelCalifornia added the rfc This describes a feature, enhancement, or optimization in broad terms and should be discussed label Sep 12, 2019
@Octogonapus
Copy link
Contributor

What if two LVGL versions require conflicting in-kernel forward declarations?

@HotelCalifornia
Copy link
Contributor Author

What if two LVGL versions require conflicting in-kernel forward declarations?

I'm not sure I can think of a situation where this would occur.

@theol0403
Copy link
Contributor

I can think of two other benefits to this.

Firstly, users would be able to compile LVGL on their own, much like okapi. This has the benefit of allowing users to upgrade LVGL themselves, or work on upgrade PRs.
The other benefit of doing this is that users would be able to modify LVGL settings to fit their needs. The most obvious application of this is enabling the debug output of LVGL, but they could also use it to enable/disable modules or make other custom tweaks.

Secondly, this allows users to completely uninstall LVGL, which can reduce download speeds if not using hot/cold. This can also allow users to provide their own custom graphics library without interfering with LVGL. I have heard some users coming from VCS to PROS that a major drawback of PROS is that LVGL is too complicated, and they would rather have the display API provided by VEXos.
What I think would be awesome is if PROS provides an alternate template available for download that users could install instead of LVGL, which provides the tmei functionality that was present in PROS 3 beta.

@TixoRebel
Copy link
Contributor

I agree with @theol0403 in that providing thin wrapper for the VEXos display API would be quite valuable for users, especially those who are coming from some other V5 programming solution and want something more similar to what they are used to.

  • disp_daemon is a static task. Same deal here. We can either forward declare the relevant static task function(s) or just decide to expose them to users in pros/apix.h.

Exposing disp_daemon in pros/apix.h seems like a good plan, especially because it could let a user create their own display library using it.

  • What about internal kernel functions that use LVGL? To my knowledge the only place the kernel refers to LVGL (aside from LLEMU, which will be addressed next) is for the VDML port unplugging/mismatch warning. We might want to consider removing this warning entirely, as it duplicates vexOS functionality (that was not present when the system was initially implemented).

Ya, this functionality seems unnecessary at this point and just causes headache for a user when they have to clear two errors.

  • What about LLEMU? LLEMU may also be refactored out into this separate template. If we make it a default template, then new projects will still work. Advanced users who modify their default templates should know that removing this will break the default PROS kernel template code.

Refactoring LLEMU into a separate template seems like a good idea anyways as some users may not use / want it.

@HotelCalifornia
Copy link
Contributor Author

Exposing disp_daemon in pros/apix.h seems like a good plan, especially because it could let a user create their own display library using it.

Sorry if I wasn't clear, but I meant the static tasks API here.

@TixoRebel
Copy link
Contributor

Exposing disp_daemon in pros/apix.h seems like a good plan, especially because it could let a user create their own display library using it.

Sorry if I wasn't clear, but I meant the static tasks API here.

Ah, well, that make sense. It would probably be a good addition. I see now that disp_daemon doesn't have anything special about it other then being a static task.

@HotelCalifornia HotelCalifornia changed the title Decouple LVGL from the kernel RFC: Decouple LVGL from the kernel Oct 18, 2019
@HotelCalifornia HotelCalifornia changed the title RFC: Decouple LVGL from the kernel LVGL from the kernel Oct 18, 2019
@HotelCalifornia HotelCalifornia changed the title LVGL from the kernel RFC: Decouple LVGL from the kernel Dec 18, 2019
@3038922
Copy link

3038922 commented Feb 24, 2020

I'd love to help.I will not take part in the world championships this year.I'm very free now.
But I didn't have
[submodule "firmware/libv5rts"] path = firmware/libv5rts url = git@github.com:purduesigbots/libv5rts.git ignore = dirty
Without this, it's hard for me to find out the mistakes.

@nathan-moore
Copy link
Member

That module is vex's private sdk, which we can't distribute.

@3038922
Copy link

3038922 commented Feb 26, 2020

https://github.com/3038922/pros-lvgl-sim
I'm almost finished, and I'll improve the adaptability in the future. Try not to change the pros API.

@HotelCalifornia
Copy link
Contributor Author

The PROS API shouldn't change at all from the user's perspective. The LVGL API might change in that we'd be able to easily integrate v6. That said, the idea is that keeping a separate LVGL template will allow us to more easily manage versioning of that library, so the user can choose whether or not they want (for example) v5.3 or v6

@WillXuCodes
Copy link
Member

Alright, so I think the general idea here is to do the following:

  • Create a template for LVGL 5.3 and in the future create one for v6 or v7.
    • This being said, I think we should create a version with and without llemu which would as Hotel mentioned, allow people to choose whether or not they want to
    • After gutting Okapilib for a cleaner template build from a pros project made by Hotel that Kunwar already tested (confirming that it works), I've created a template for LVGL 5.3 with the llemu (still need to upload it to github as a repo).
    • Still need to decouple llemu from this itself though.
  • Remove all LVGL features from the kernel. This is already done, it's the lvgl-decouple branch and builds/compiles properly. Still needs testing though with the LVGL template(s) mentioned above.
  • Potentially add native support for some of VEXos's graphical features. While this is relatively easy in itself, figuring out how to resolve conflicts between this and lvgl as well as what we should name the seperate namespace instead of lcd:: is something that should be discussed.

@nathan-moore
Copy link
Member

Presumably llemu would be in its own template, or with lvgl's (since I don't know what you mean by a version with and without llemu).

The other thing that comes to mind is to make lvgl thread safe. Currently, it isn't, and that's what I think is the root cause of some problems that popped up ie purduesigbots/liblvgl#8. Otherwise sounds great.

@WillXuCodes
Copy link
Member

Yeah, that's exactly what I meant. I presumed that llemu users would need lvgl so I've created one template with just lvgl, and one that includes both lvgl and llemu.

@theol0403
Copy link
Contributor

For vexOS graphical features, I think the plan is to have a template that is mutually exclusive to the LVGL template. Then when the user chooses the vexOS template, there is no LVGL present in the entire project. I think it is reasonable to expect users to use one or the other, as LVGL takes over the entire screen as soon as it is used. However, it is also a possibility to allow users to have both templates installed, and just have users know that as soon as they use any LVGL API that it will take over the screen.

I think LLEMU should either be a part of the LVGL template or be its own default template which has the LVGL template as a dependency. I don't really like having a LVGL only and LVGL + LLEMU template, as those also have to be mutually exclusive (if the LLEMU + LVGL template is default, you need to uninstall it and install the LVGL only template, vs just uninstalling the LLEMU template).

Finally, the vexOS graphical library PROS API has already been done for us! c651608

@WillXuCodes
Copy link
Member

The rationale behind packaging lvgl and llemu together in one library so students wouldn't ask why their llemu libraries weren't working if they didn't install the llemu template. Of course, if we make it clear in the docs that llemu won't work without the lvgl libraries this won't be an issue. I'm thinking we could have 3 options for users- the two mentioned originally as well as the library without lvgl and just llemu. The library with llemu + lvgl could act as a shortcut for users that want to cut to the chase if they need be.

I was going to make the vexOS template the default, so newer users wouldn't believe that the lack of it installed by default somehow meant we neglected to have any touchscreen functionality in pros. However, this is more of a minor point and we could just not do this at all and focus on the lvgl side of things for now if we can get over this.

@theol0403
Copy link
Contributor

theol0403 commented Aug 6, 2020

ask why their llemu libraries weren't working if they didn't install the llemu template.

I was thinking a default PROS project would contain separate LVGL and LLEMU templates by default. I don't think TMEI should be the sole default, though it could be default and coexist with LVGL.

That way PROS will have full functionality by default and it is easy for users to remove templates they do not want.

If LVGL+LLEMU combined template was installed by default, then to remove LLEMU you would have to remove the template and install the LVGL only one (which is an extra step). It also might be a pain to maintain - make a change to LVGL and you have to publish two new templates.

By having all the components separate, you could have a custom LVGL config/version and that would not affect LLEMU (unless breaking changes) or require two templates made.

But yeah this is minor and we can decide this later.

@edjubuh
Copy link
Member

edjubuh commented Aug 6, 2020

that would not affect LLEMU (unless breaking changes)

I am under the impression that we are not updating lvgl regularly because of breaking changes

@theol0403
Copy link
Contributor

No, but if LVGL 7 is released for example (or even an incremental version of 6), I imagine a new template could be created to allow users to upgrade at their leisure. Maintaining two streams of templates might be a chore.

@WillXuCodes
Copy link
Member

Not to mention another reason why this change is happening is because we're sick of having to wait longer than we have to everytime we compile the kernel.

@edjubuh
Copy link
Member

edjubuh commented Aug 6, 2020

I'm not against decoupling. Just wanted to point out that breaking changes happens on LVGL frequently, even on minor updates. Maybe the API has stabilized now.

@HotelCalifornia
Copy link
Contributor Author

HotelCalifornia commented Aug 6, 2020

Theo and will are right, the main purpose of moving LVGL into its own template is so that we can upgrade it whenever without having to push an update to the kernel. Also members of the community would be more able to help with that effort as libv5rts would no longer be a hard dependency.

LVGL and LLEMU would go into the same liblvgl template, and any sort of TMEI or vexOS graphics-wrapping functionality would sit in the kernel as part of our public API.

I have a preliminary version of the 5.3 template that seems to work, however I can't currently access it as my computer's motherboard was sent in for RMA. As soon as I can get back to the project, I'll push what I have to a repo under the purduesigbots org.

@WillXuCodes
Copy link
Member

WillXuCodes commented Aug 10, 2020

So I'm writing another layer on top of TMEI right now, and there's two ways we could go about doing some of this in terms of API and I'd like to get some opinions on this:

  1. We could have users set the color of their "pen" to a certain color and have subsequent functions (drawing a circle, line, etc.) use that color to draw on the screen.
  2. We could have users set the color of those functions mentioned above using a parameter on each function.

@WillXuCodes
Copy link
Member

WillXuCodes commented Aug 10, 2020

I personally like #1 better from a user prespective. Reduces the amount of times I have to put color as a parameter. Might not be thread safe though but I'd like to believe that we can trust our users with that.

@nickmertin
Copy link
Contributor

I also like the first one better, for the same reason.

Might not be thread safe though but I'd like to believe that we can trust our users with that.

Yeah, I think the docs should be clear on that, but as long as we can make sure it won't crash (i.e. the only effect might be the colour changes), then it's fine to leave the rest up to the users.

@HotelCalifornia
Copy link
Contributor Author

gonna be honest: I don't really think we need to worry about doing anything beyond the barebones vexOS API wrapper for kernel-included graphics capability. if users want more, they can either use LVGL or make a library of their own (@WillXuCodes maybe the work you've done in that regard can be such a library? just a thought)

@kisvegabor
Copy link

Hi,

I'm from LVGL and just noticed this issue. We already have LVGL v8 and its API will stay for a long time.
I'm new to PROS kernel but, if you have any questions about LVGL or have any suggestions about how LVGL can support easier porting, feel free to tag me here or open an issue in LVGL's repo.

@WillXuCodes
Copy link
Member

Hi kisvegabor,

Thanks for reaching out! Our current plan is not to completely remove LVGL from the Kernel, but to port it as a library that's easier for us to upgrade and easier for users to use different versions for. We'll definently be in touch if we have questions or suggestions.

@Haxered
Copy link

Haxered commented Feb 9, 2022

I was just wondering when PROS 4 is set to release. I would really like to use LVGL 8 with pros. I've tried to upgrade the library manually but it gives me a Linking error. Is there any way I could use LVGL 8 with the current pros version? I would literally do anything.

@WillXuCodes WillXuCodes changed the title RFC: Decouple LVGL from the kernel RFC: Decouple LVGL and Make LVGL Template Jul 26, 2022
@WillXuCodes WillXuCodes changed the title RFC: Decouple LVGL and Make LVGL Template PROS 4: Decouple LVGL and Make LVGL Template Aug 25, 2022
@WillXuCodes
Copy link
Member

With the LVGL Template underway I'm just going to close this issue since there's not much to discuss anymore and the repo for lvgl specific issues is already up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rfc This describes a feature, enhancement, or optimization in broad terms and should be discussed
Projects
None yet