WLED icon indicating copy to clipboard operation
WLED copied to clipboard

platform.ini cleanup + version bumps

Open TripleWhy opened this issue 1 year ago • 9 comments

Goals

  1. Make the platform.ini file easier to understand and read.
  2. Being able to use more recent compiler and c++ versions (in all environments, instead of most environments). This will enable future improvements, that I am working on.

Changes

  • make formatting more uniform
  • organize environments hierarchically
  • remove duplications
  • upgrade platform versions
  • remove some obsolete dependencies
  • upgrade some dependencies while at it
  • upgrade c++ version from gnu++11 to gnu++17 (fastled doesn't compile without gnu extensions)
  • remove debug symbols from the esp32 release builds

Discussion

Platform Version Upgrade

Most environments were using relatively recent platform versions, but the base esp32 configuration was not. Upgrading it does increase image size. But that is a problem we have anyway, because we do have envs that use more recent platform versions. This change just results in fewer differences between environments. It also increases the minimum gcc version the code has to compile with, therefore enabling better features and more recent c++ versions.

Dependencies:

The original LittleFS dependency was removed, because all platforms are now recent enough to include it anyway.

The other removed dependency is framework-arduinoespressif32 @ https://github.com/Aircoookie/arduino-esp32.git#1.0.6.4 that was used by the base esp32 configuration, but not all environments. I removed it because it is incompatible with the more recent platform version. I could however not figure out what it was good for originally, so hopefully this doesn't break anything? As far as I can tell, WLED runs fine without it on the ESP-WROOM-32 board. (Most other boards didn't use it.)

Further Upgrade Options:

This PR brings the minimum GCC version from 5.2 to 8.4. It is possible to get it up to 12.2 by adding a line such as:

platform_packages = toolchain-xtensa-esp32@~12.2

That works, I tried it. It would however require specifying different toolchains for each architecture (I think).

TripleWhy avatar Jul 07 '24 15:07 TripleWhy

Hi @TripleWhy,

About the esp32 platform version - sorry this is a no-go from my point of view. Same for changing gcc version to diverge from the versions included in respective arduino-esp32 release.

For esp32 4MB, we first need a solution to reduce firmware size before upgrading the framework.

softhack007 avatar Jul 07 '24 21:07 softhack007

@TripleWhy maybe you did not recognize this - the platformio.ini "high level" structure is correct, and it was chosen carefully. We need an independent base environment for each chip model - 8266, esp32, esp32-s3, esp32-s2, esp32-c3. Two years ago we tried to base all esp32 "variants" on the same build_flags and lib_deps, and it was a huge disaster. This is why we have largely independent top level environments for [esp32], [esp32-S3], etc. Please do not touch this part.

You may of course propose optimizations for the lower level [env:....] environments. Especially the use of "extends" is very welcome. Also minor readability improvements are welcome.

If you change the main environments (without "env" prefix), please also perform exhaustive functional testing on the environments/boards that inherit from the main sections. "It compiles" is not good enough as a justification 😉 when changing the main sections.

softhack007 avatar Jul 07 '24 21:07 softhack007

Any change to platformio.ini for platforms or packages or toolchains or boards or base environments will require thorough testing on several different boards of all supported MCU variants. I cannot approve this for 0.15 in any form but it may be useful for 0.16 when we start working on it.

blazoncek avatar Jul 09 '24 06:07 blazoncek

Thanks for the insight everyone. I should not have mixed refactoring and version changes, so that there can be independent discussions about those topics.

Therefore I propose the following:
If I can produce a new platform.ini without version changes that I can prove produces the same result as the current one, would you accept that?

Based on that, I would then later open a new discussion about versions. (So I am deliberately not replying to version related comments here.)

TripleWhy avatar Jul 09 '24 11:07 TripleWhy

Therefore I propose the following: If I can produce a new platform.ini without version changes that I can prove produces the same result as the current one, would you accept that?

Hi @TripleWhy thanks for your proposal, this makes a lot of sense to me.

Before we go again into building "heads with nails" (instead of nails with heads), maybe let's see what are our needs.

From my perspective - as the evil genius who drove new MCU support for some time - I think we need this as a generic structure:

  • [env] -> generic setting for platformIO, like framework=arduino. Not sure if both [common] and [env] are needed?
  • [common]-> flags and libs for WLED in general.
  • |- [esp8266] -> everything specific to 8266
  • |- [esp32_base] (or any better name) flags, libs, partitions, that are the same for all esp32 variants. Maybe include platform. Also collect flags like AR_lib_deps
  • |--> next 6 targets inherit from [esp32_base]
  • |--- [esp32] -> classic esp32, arduino-esp32 v1.x (legacy)
  • |--- [esp32_V4] -> classic esp32, but using newer arduino-esp32 v2.x
  • |--- [esp32s3]
  • |--- [esp32s2]
  • |--- [esp32c3]
  • |--- [esp32c6] (maybe supported in WLED 0.16.)

The last 6 targets can be the baseline for user's specific buildenvs.

The reason is that some MCUs will always need special flags. If you follow a strict top-down design, this will easily lead into conflicting flags. So I prefer to have esp32, s3, s2, c3 (and later c6 maybe) all side-by-side on the second or third level.

Definition of done

  • standard firmware is the same when build with old or new platformIO.ini (binary compare)
  • randomly picked custom buildenvs for the previous platformIO.ini still compile without errors using the new platformio.ini
  • maybe also check if 3rd party build tools - like the tool from @wled-install - are still working correctly.

Comments welcome 😃

softhack007 avatar Jul 09 '24 19:07 softhack007

The reason is that some MCUs will always need special flags. If you follow a strict top-down design, this will easily lead into conflicting flags. So I prefer to have esp32, s3, s2, c3 (and later c6 maybe) all side-by-side on the second or third level.

I see the point, but:

  1. My impression was that you are supposed to create an override file instead of modifying the platform.ini to customize your build. So in the end is only matters what is effectively inside the env: sections. For that it doesn't matter if the content came from an extend or from repetition. While that doesn't matter for users who want to create their own builds, fewer repetitions (almost) always makes the maintainer's job easier.
  2. Even with a hierarchical structure, you can always overwrite the settings anyway either by repeating, or for compiler flags, you can simply append a contradicting flag, as only the latest one is considered.

That being said, the hierarchy you suggest might actually make sense from a logical point of view, if you consider e.g. ESP32 and ESP32-C3 different chips. (I guess they are.) Turns out the issue here is simply the naming. I therefore suggest renaming the env:esp32 (and env:esp32_v4) to something like env:esp32_classic(_v4). Or maybe to find a more technical term wroom or lx6 instead of classic (would that be correct?).

TripleWhy avatar Jul 09 '24 20:07 TripleWhy

Just sharing my experience with Platformio setup for a big project. Do not change many stuff in one step. Platformio is highly complex under the hood and has its bugs. The extensive use of extends is in my experience not what I would do. Been there and had "fun" and simplified the very elegant looking setup. Using scripts has its bugs (in Platformio) too. I avoid to change a working setup. In general "overriding" previous settings can have very strange unwanted side effects.

If strange compile errors occur (header files not found and such stuff) after a Platformio setup change. There is a problem "somewhere" in Platformio. The issue is completely unrelated!!

Edit: Forgot... Be very careful with changing lib deps. It is hell!!!!

Edit2: Don't do this

Being able to use more recent compiler and c++ versions (in all environments, instead of most environments). This will enable future improvements, that I am working on.

You will run in very ugly issues. There are reasons why a specific version is choosen. Not a random choice. I did the changes needed to compile the gcc 8.x toolchains on apple arm. This changes where later used from espressif. What I wanna say, was in deep in building gcc. Earlier versions do have a lot of bugs in the new feature set. Starting with gcc 12.2 for esp32x it is safe to use gnu++17 setting.

Jason2866 avatar Jul 12 '24 13:07 Jason2866

Thanks @Jason2866 what you say makes a lot of sense to me.

The extensive use of extends is in my experience not what I would do. Been there and had "fun" and simplified the very elegant looking setup.

While I like how "extends" improves readability, its also true it makes understanding what's really going on a nightmare sometimes.

From my own experience: I had a platformio.ini with lots of inheritance by extends=. The problem was, in one build env there was a flag that should not be there, while two other flags - that were visible in env specific build_flags - were missing. I was like WTF and it took me two days to understand what's going on under the hood. In the end, I used advanced -> compilation database just to see the flags that were actually passed to gcc. It turned out that one inheritance path was wrong (explaining the extra build flag), and a deliberate "build_unflags=" from another inheritance path removed the flags that should be there. :shipit:

softhack007 avatar Jul 12 '24 20:07 softhack007

extends are bane and boon at the same time.

I used to avoid them but it was PITA to maintain platformio_override.ini with 20+ environments.

I am now using them extensively but have carefully crafted hierarchy in my overrides. So my take would be to avoid extends in base platformio.ini but give commented examples in sample override file.

Disclaimer: I am not a developer and never delved deep into how PIO works so all my knowledge comes from working or failed experiments.

blazoncek avatar Jul 13 '24 05:07 blazoncek