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

Move LVGL gluecode back into main repository #61300

Merged
merged 1 commit into from Aug 16, 2023

Conversation

faxe1008
Copy link
Collaborator

@faxe1008 faxe1008 commented Aug 8, 2023

Addresses: #60868

The related PR in the module: zephyrproject-rtos/lvgl#45.

@zephyrbot zephyrbot added manifest manifest-lvgl DNM This PR should not be merged (Do Not Merge) labels Aug 8, 2023
@zephyrbot
Copy link
Collaborator

zephyrbot commented Aug 8, 2023

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
lvgl zephyrproject-rtos/lvgl@5da257f zephyrproject-rtos/lvgl@8a6a2d1 (zephyr) zephyrproject-rtos/lvgl@5da257f7..8a6a2d1d

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@faxe1008
Copy link
Collaborator Author

faxe1008 commented Aug 9, 2023

@fabiobaltieri I merged the changes from pull/44 into here.

Building the sample works fine for me, but there are still some CI issues and I have no idea on how I can resolve them:
CI issue 1
CI issue 2

Simply put I need to somehow source the Kconfig from the module but don't know what the right way to do it is, considering I also have code in the zephyr main repo. Do you have any insights here :^) ?

@fabiobaltieri
Copy link
Member

fabiobaltieri commented Aug 11, 2023

Simply put I need to somehow source the Kconfig from the module but don't know what the right way to do it is, considering I also have code in the zephyr main repo. Do you have any insights here :^) ?

Yeah so you want the module system to only include the Kconfig when it's there, just drop kconfig-ext: True from module.yml in the LVGL module PR and that'll make build/Kconfig/Kconfig.modules take care of the whole story.

diff --git a/zephyr/module.yml b/zephyr/module.yml
index e0e7d676..38ef2dd2 100644
--- a/zephyr/module.yml
+++ b/zephyr/module.yml
@@ -2,5 +2,4 @@
 name: lvgl
 build:
   cmake-ext: True
-  kconfig-ext: True
   kconfig: Kconfig

Then the Kconfig that is always there needs to have stub symbols with defined types for everything that is overridden at board level, apply this:

diff --git a/modules/lvgl/Kconfig b/modules/lvgl/Kconfig
index 43b187abee..9d1b1bc1da 100644
--- a/modules/lvgl/Kconfig
+++ b/modules/lvgl/Kconfig
@@ -11,9 +11,14 @@ config LVGL
 
 if LVGL
 
-source "${ZEPHYR_LVGL_MODULE_DIR}/Kconfig"
+config LV_USE_MONKEY
+       bool
+
+config LV_DPI_DEF
+       int
 
 config LV_CONF_SKIP
+       bool
        default n
 
 config APP_LINK_WITH_LVGL
@@ -25,7 +30,7 @@ config APP_LINK_WITH_LVGL
          issues for 'app'.
 
 config LV_Z_USE_FILESYSTEM
-       bool "Enable file system"
+       bool "LVGL file system support"
        depends on FILE_SYSTEM
        default y if FILE_SYSTEM
        help

@fabiobaltieri
Copy link
Member

fabiobaltieri commented Aug 11, 2023

By the way, you can run the compliance checks locally with ./scripts/ci/check_compliance.py, something like ./scripts/ci/check_compliance.py -c origin/main.., saves you from pushing and waiting for CI to run it for you.

@faxe1008
Copy link
Collaborator Author

Thanks @fabiobaltieri for the hints, apart from a checkpatch false positive everything builds fine now.

@faxe1008 faxe1008 marked this pull request as ready for review August 14, 2023 06:29
@fabiobaltieri
Copy link
Member

Thanks for taking care of this, looks good, should be a single commit though so if a bisect lands on the first one it builds correctly.

@faxe1008
Copy link
Collaborator Author

Squashed the commits down :^)

modules/lvgl/lv_conf.h Outdated Show resolved Hide resolved
modules/lvgl/lvgl_fs.h Outdated Show resolved Hide resolved
west.yml Outdated Show resolved Hide resolved
Moves back the module specific gluecode into the main repository

Signed-off-by: Fabian Blatz <fabianblatz@gmail.com>
@faxe1008
Copy link
Collaborator Author

Adjusted all include guards, updated revision :^)

@zephyrbot zephyrbot removed the DNM This PR should not be merged (Do Not Merge) label Aug 15, 2023
@fabiobaltieri
Copy link
Member

@gmarull @jfischer-no @danieldegrasse any second approval for this? The CI fail is a false positive.

Copy link
Collaborator

@danieldegrasse danieldegrasse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking this on- looks good to me, tested on an RT1170 with the LVGL benchmark and widgets demos and everything seems functional

@carlescufi carlescufi merged commit 2e2163c into zephyrproject-rtos:main Aug 16, 2023
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants