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

PlatformIO instructions are incomplete #220

Closed
cmumford opened this issue Oct 24, 2020 · 11 comments
Closed

PlatformIO instructions are incomplete #220

cmumford opened this issue Oct 24, 2020 · 11 comments
Labels
stale This will not be worked on

Comments

@cmumford
Copy link
Contributor

I'm new to PlatformIO, but I'm pretty sure that the platformio instructions in README.md are incorrect. I read through issue #168 and it only recommends the following:

  1. Start with lv_platformio
  2. Modify platformio.ini
  3. Modify main.cc

I think it is ambiguous whether it is necessary to create the git submodule in the lv_platformio project is required as described in Build this repository standalone and run the demo. If this was done, then this would result in two copies of lvgl. One in //components/lv_port_esp32/components/lvgl/lvgl/, and a second in //lib/lvgl/. If the submodule is not created, then there is a missing step to add lv_port_esp32 as a dependency (lib_deps) in platformio.ini.

There is a comment in issue 168 which says that following the README.md steps successfully builds - which it does, but the default app doesn't use any drivers and doesn't display anything.

Maybe somebody with more PlatformIO knowledge can see what I might be doing wrong, but from what I can see this project is not compatible (or ready to be used by) PlatformIO.

cmumford added a commit to cmumford/lv_port_esp32 that referenced this issue Oct 25, 2020
When LV_LVGL_H_INCLUDE_SIMPLE is not defined (the default) then
`"lvgl/lvgl.h"` is included, else `"lvgl.h"` is included.
This is for other build systems (like PlatformiIO) which setup
build paths differently than esp-idf. More info in issue lvgl#220.
cmumford added a commit to cmumford/lv_port_esp32 that referenced this issue Oct 25, 2020
PlatformIO compiles using a different include path strategy than
esp-idf. If PLATFORMIO is defined then `"lv_core/lv_refr.h"` will
be included, otherwise `"lvgl/src/lv_core/lv_refr.h"`. More info
in issue lvgl#220.
@cmumford
Copy link
Contributor Author

cmumford commented Oct 25, 2020

I managed to get my project running with lv_port_esp32. Here's what I had to do:

  1. Checkout lv_port_esp32 into the //lib directory.
  2. Make the following changes to lv_port_esp32:
    1. Include lvgl.h differently, PR Including "lvgl.h" if LV_LVGL_H_INCLUDE_SIMPLE is defined. #221
    2. Include lv_refr.h diffently, PR Including lv_core/lv_refr.h differently form platformio. #222.
    3. Added a library.json file (no PR yet - more on that later).
  3. Made the following changes to my app's project:
    1. Modified my platformio.ini file to have "lv_port_esp32" in the "lib_deps" section.
    2. Copied lib/lv_port_esp32/components/lvgl/lv_conf. to src/lv_conf.h
    3. Copied the three Kconfig files from this project (skipping examples) to my project.
    4. Ran platformio run -t menuconfig, and set values as described by https://github.com/lvgl/lv_port_esp32#configuration-options.
    5. Added the LVGL code from main.c into my project's application.

Here is what I'd like to do to complete support for PlatformIO for this project - as far as I can tell lv_port_esp32 really isn't yet compatible with PlatformIO.

  1. Land the two PR's above.
  2. Complete and land a PR for library.json. This is necessary for platformio to see this project as a library project and to know what files to build.
  3. Modify all driver files to ifdef out their code if the appropriate constant isn't defined. For example, the contents of ili9341.c will be surrounded by:
    #if defined(CONFIG_LV_TFT_DISPLAY_CONTROLLER_ILI9341)
    ...
    #endif
    This is necessary because library.json doesn't have the ability (I believe) to selectively compile files, so they will need to be a no-op - except for the drivers turned on in sdkconfig.
  4. Hopefully come up with an alternative to making a copy of lv_conf.h.
  5. Hopefully come up with an alternative to copying the Kconfig files.
  6. Modify the README platformio section. I believe it should still say "go to lv_platformio and follow instructions there".
  7. Add the actual LVGL+PlatformIO+ESP32 instructions to the lv_platformio project.

If anybody knows PlatformIO better, please let me know if I'm not seeing an easier solution, but this is the cleanest solution I can come up with at the moment.

@kisvegabor
Copy link
Member

Hi Chris,

Thanks for sharing your experiences. The things you have pointed out seems correct.

In the very near future, we will make lvgl and lv_examples directly work as an ESP component. lvgl was updated to work with Kconfig (Kconfig file will be added soon) and without lv_conf.h, that is you can add LVGL defines to platfromio.ini. (it should solve "Hopefully come up with an alternative to making a copy of lv_conf.h.").

Finally, the drivers part from this repo will be moved to its own repo. The changes you have suggested for the driver part seems valid to me. However, @C47D is the maintainer of this repo and he knows it much better. 🙂

cmumford added a commit to cmumford/lv_port_esp32 that referenced this issue Oct 27, 2020
This change wraps driver header/source files in the appropriate
constants to effectively make them empty if that driver is not
used in this build.

The ESP-IDF build system is smart enough to conditionally build
source files depending on project settings (see CMakeLists.txt
in drivers folders). PlatformIO is not, and always builds every
file. This change effectively makes those files no-ops
which serves the same purpose and makes this project able to
compile with PlatformIO (and other build systems).

This addresses, but does not entirely fix, issue lvgl#220.
@cmumford
Copy link
Contributor Author

OK. user @davidjade was very helpful on PR #225, and I think this would be a more clear way to describe how to build this project using PlatformIO.

  1. Follow the configuration steps above in Build this repository standalone and run the demo - i.e. steps 1-6.
  2. Create the PlatformIO using the desired board. In this example we are using "esp-wrover-kit":
    pio project init --board esp-wrover-kit
    rmdir src # src/ not needed - this project uses main/
    This will create a platformio.ini file like this:
    [env:esp-wrover-kit]
    platform = espressif32
    board = esp-wrover-kit
    framework = arduino
  3. Specify the src_dir setting in platformio.ini to be "main":
    [platformio]
    src_dir = main
    
    [env:esp-wrover-kit]
    platform = espressif32
    board = esp-wrover-kit
    framework = arduino
  4. Change the framework in platformio.ini to be "espidf"
    [platformio]
    src_dir = main
    
    [env:esp-wrover-kit]
    platform = espressif32
    board = esp-wrover-kit
    framework = espidf
  5. [Optional step] Add new project files to source control:
    git add platformio.ini include/ lib/ test/
  6. Build as desired. e.g. with the command-line using platformio run, or with VSCode.

I think it we should keep the reference to the lv_platformio project as a more complete example.

I still think that ideally PlatformIO projects should utilize Platform IO Library Management to source project dependencies (LVGL, lv_port_esp32, etc.). To avoid confusion, But, I think it's best to have that discussion in a different bug - probably after @C47D finishes his lv_drivers refactoring.

@kisvegabor
Copy link
Member

@cmumford Thanks for sharing this. I wonder if we can add these environment settings to platform.ini file lv_platformio.

@stale
Copy link

stale bot commented Nov 24, 2020

This issue or pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale This will not be worked on label Nov 24, 2020
@stale stale bot closed this as completed Dec 1, 2020
@tlechman49
Copy link
Contributor

Is more work being done on this topic? Should I just follow steps as @cmumford described? The instructions for set up with platformio are still incomplete and this post doesn't seem like it should be left closed.

@C47D
Copy link
Collaborator

C47D commented Dec 8, 2020

Hi @tlechman49, can you try the @cmumford steps? If they work please let us know, on the meantime I reopen the issue.

@C47D C47D reopened this Dec 8, 2020
@stale stale bot removed the stale This will not be worked on label Dec 8, 2020
@cmumford
Copy link
Contributor Author

cmumford commented Dec 8, 2020

I'm happy to put together a PR with these changes if @tlechman49 concludes they are correct.

@tlechman49
Copy link
Contributor

So I actually just posted a pull request ( #234 ) for this that worked fine for me. If someone else tests that, I guess further notes should be added on how to set the .ini file in the readme and then this would be solved.

@stale
Copy link

stale bot commented Dec 31, 2020

This issue or pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale This will not be worked on label Dec 31, 2020
@kisvegabor
Copy link
Member

The PR is merged so I close this issue.

Thank you @tlechman49

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants