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

Update lvgl 8.3.11 #47

Merged
merged 95 commits into from
Jan 22, 2024

Conversation

faxe1008
Copy link
Contributor

@faxe1008 faxe1008 commented Sep 28, 2023

Update LVGL module to version 8.3.11. Full release notes here:
CHANGELOG.md.

Substantial fixes:

  • fix(disp): fix memory leak lv_scr_load_anim with auto_del and time=0 1caafc5
  • fix(btnmatrix): fix array out of bounds addressing with groups and no buttons edd5ad2
  • fix(chart): fix lv_chart_get_point_pos_by_id f9ffcc9
  • fix(chart): fix division by zero if there are no ticks 67b3011

@faxe1008 faxe1008 marked this pull request as draft September 28, 2023 08:21
@faxe1008 faxe1008 changed the title Update lvgl 8.3.10 DNM Update lvgl 8.3.10 Sep 28, 2023
@faxe1008
Copy link
Contributor Author

This commit introduced a regression in regards to background opacity behaviour when loading screens with FADE_ANIM for me:

commit ebd3537c2c92de1532a4b4465f4f08f8f5ff556b
Author: _VIFEXTech <vifextech@foxmail.com>
Date:   Mon Aug 14 13:57:19 2023 +0800

    feat(style): backport opa_layered

@faxe1008 faxe1008 changed the title DNM Update lvgl 8.3.10 DNM Update lvgl 8.3.11 Dec 8, 2023
@faxe1008
Copy link
Contributor Author

faxe1008 commented Dec 8, 2023

Yesterday v8.3.11 was released. I added the respective new commits to this PR. So far I could not spot any regressions.

@pdgendt
Copy link
Collaborator

pdgendt commented Dec 8, 2023

Do we still need a zephyr branch to merge to, or could we simply track upstream releases?

faxe1008 added a commit to faxe1008/zephyr that referenced this pull request Dec 8, 2023
Related to zephyrproject-rtos/lvgl#47

Signed-off-by: Fabian Blatz <fabianblatz@gmail.com>
@fabiobaltieri
Copy link
Member

Do we still need a zephyr branch to merge to, or could we simply track upstream releases?

Well we need a branch to merge to, are you proposing to ditch the repo entirely? Pretty sure we don't what that even if it follows upstream 1:1, it's useful to have anyway in case we want to diverge temporarily and converge back down the line.

@pdgendt
Copy link
Collaborator

pdgendt commented Dec 8, 2023

Well we need a branch to merge to, are you proposing to ditch the repo entirely? Pretty sure we don't what that even if it follows upstream 1:1, it's useful to have anyway in case we want to diverge temporarily and converge back down the line.

Not ditching, simply follow upstream 1:1, like we do for openthread.

@fabiobaltieri
Copy link
Member

Not ditching, simply follow upstream 1:1, like we do for openthread.

Ok so you mean: drop the zephyr branch, instead use a main branch that tracks upstream? That'd be cool... it could still be called zephyr :-)

@pdgendt
Copy link
Collaborator

pdgendt commented Dec 8, 2023

Ok so you mean: drop the zephyr branch, instead use a main branch that tracks upstream? That'd be cool... it could still be called zephyr :-)

Right, but now we have to keep adding merge commits simply for the zephyr folder?

@fabiobaltieri
Copy link
Member

fabiobaltieri commented Dec 8, 2023

Oh I see, yeah it'd make sense to use main, or maybe just tag it so git does not drop the refs and then force push it. Probably not worth it.

@faxe1008 faxe1008 marked this pull request as ready for review December 8, 2023 13:30
@faxe1008 faxe1008 changed the title DNM Update lvgl 8.3.11 Update lvgl 8.3.11 Dec 8, 2023
Kconfig Outdated Show resolved Hide resolved
@faxe1008
Copy link
Contributor Author

faxe1008 commented Dec 8, 2023

@pdgendt @fabiobaltieri Sure it would be doable to set up a branch which just tracks upstream. The differences between upstream and our fork mostly comes down to Kconfig changes. Btw. what I also found is that upstream LVGL also has zephyr module.yaml already 🙃 https://github.com/lvgl/lvgl/blob/master/env_support/zephyr/module.yml

Maybe we can sync our module file to upstream and have it be part of v9.0, so that we can eventually ditch the zephyr folder?

@fabiobaltieri
Copy link
Member

Maybe we can sync our module file to upstream and have it be part of v9.0, so that we can eventually ditch the zephyr folder?

Ideal.

@pdgendt
Copy link
Collaborator

pdgendt commented Dec 8, 2023

The module file is probably outdated if nobody cares to submit PRs. Maybe we can upstream already for v8?

@faxe1008
Copy link
Contributor Author

@fabiobaltieri @pdgendt

The module file is probably outdated if nobody cares to submit PRs. Maybe we can upstream already for v8?

The project is currently finalizing v9 - I assume that there will not be another v8 release in the near future. Also one issue is that the module.yaml lives in a env_support/zephyr folder which is not supported by west, at least from I can tell.

@faxe1008
Copy link
Contributor Author

faxe1008 commented Jan 3, 2024

Ping @fabiobaltieri @pdgendt do you maybe have an idea if its possible that the modules module.yml lives inside the env_support folder and not within root of the repo?

@fabiobaltieri
Copy link
Member

fabiobaltieri commented Jan 3, 2024

Ping @fabiobaltieri @pdgendt do you maybe have an idea if its possible that the modules module.yml lives inside the env_support folder and not within root of the repo?

It looks like the location is hardcoded https://github.com/zephyrproject-rtos/zephyr/blob/main/scripts/zephyr_module.py#L179-L180 so with the current code base that is not possible, that said, it may be reasonable to expand the search path, maybe look for a module.yml file in any directory named zephyr in the module or something like that. The problem is that then you could mess things up with nested repositories or other weird corner cases so it needs some thinking. Are you thinking about trying to go 100% in sync with upstream?

@faxe1008
Copy link
Contributor Author

faxe1008 commented Jan 4, 2024

@fabiobaltieri Thanks for your confirmation. 100% is not feasable since there are Kconfig defaults that we have set differently. I am content to have the merging strat etc. as is. I think that @pdgendt raised the question of tracking 1:1 originally.
As I see it's not really possible to track upstream directly as we already diverged slightly and from what I see other modules also still perform merges into a dedicated zephyr branch.

Anyways if there is feasable solution I'd love to hear it. This should not be a blocker for this update imo.

@fabiobaltieri
Copy link
Member

Anyways if there is feasable solution I'd love to hear it. This should not be a blocker for this update imo.

If that's the only delta from upstream I think it may make sense to try and push to make the module file detection a bit smarter, but if we are going to have other patches on top of upstream anyway it's probably not worth the hassle IMO.

@fabiobaltieri
Copy link
Member

fabiobaltieri commented Jan 22, 2024

@faxe1008 is this good to go? let's go!

@fabiobaltieri fabiobaltieri merged commit 2b76c64 into zephyrproject-rtos:zephyr Jan 22, 2024
6 checks passed
carlescufi pushed a commit to zephyrproject-rtos/zephyr that referenced this pull request Jan 22, 2024
Related to zephyrproject-rtos/lvgl#47

Signed-off-by: Fabian Blatz <fabianblatz@gmail.com>
coreboot-org-bot pushed a commit to coreboot/zephyr-cros that referenced this pull request Feb 3, 2024
Related to zephyrproject-rtos/lvgl#47

(cherry picked from commit 2a11c64)

Original-Signed-off-by: Fabian Blatz <fabianblatz@gmail.com>
GitOrigin-RevId: 2a11c64
Change-Id: I87c23131558fd25c34de77fcb26f44fdf292814a
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/zephyr/+/5227908
Reviewed-by: Tristan Honscheid <honscheid@google.com>
Commit-Queue: Tristan Honscheid <honscheid@google.com>
Tested-by: Tristan Honscheid <honscheid@google.com>
Tested-by: ChromeOS Prod (Robot) <chromeos-ci-prod@chromeos-bot.iam.gserviceaccount.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.