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

Only create bin.gz files for esp8266 #3859

Merged
merged 10 commits into from
Mar 29, 2024
Merged

Only create bin.gz files for esp8266 #3859

merged 10 commits into from
Mar 29, 2024

Conversation

w00000dy
Copy link
Contributor

This implements the wishes of @srg74. 😄

Now the bin.gz files are only generated when building for espressif8266.
I also removed the firmware folder because it contained the same file as the release folder and it was the same file you find in the .pio/build folder.
If there was a reason for the firmware folder to exist that I missed, let me know, but I can't think of one.

I also optimized output_bins.py because there was some duplicate code and some unnecessary code.
For example, checking if the file exists (and then deleting it if it does) is unnecessary because if it exists it will be overwritten, so there is no need to delete it first.

I also wanted to create a separate folder for the bin.gz files, but then the Upload Artifacts action does not put the files directly into the artifact, but also includes the path in the artifact.

With separate folder for bin.gz files Without separate folder for bin.gz files
grafik grafik

So I'm not sure what you prefer. I now leave the bin.gz files in the release folder, so we have the binaries right in the artifacts.

If you build an environment without the WLED_RELEASE_NAME build flag, you can find the firmware binary in the .pio/build folder.

If there is anything you want me to improve or change, just let me know! 🙂

@blazoncek
Copy link
Collaborator

Actually all ESP8266 would benefit from .gz files.

@w00000dy
Copy link
Contributor Author

So I should include bin.gz files in all esp8266 artifacts except esp01?

@srg74
Copy link
Contributor

srg74 commented Mar 28, 2024

Very useful optimization. I think compressed files only needs esp01 and esp02 builds (sp501, sp511), everything else is useless. Thanks for a great job.

@w00000dy
Copy link
Contributor Author

Afaik bin.gz is not needed for esp01, because it does not work with OTA.
@dosipod told me this in #3718:

No gz is only needed for esp02 ( 2MB ) when using OTA , ESP01 1MB does not work with OTA so gz is useless . The normal esp8266 4MB OTA will work without gz

But I can also include it for the nodemcu builds.
What would be the benefit of that?

@blazoncek
Copy link
Collaborator

AFAIK ESP32 (all of them) lacks the code to process GZ files when uploaded OTA and will complain about corrupt files if you try to upload GZ. That, IMO, is due to different way flash is organised.
ESP8266 on the other hand can handle GZ files and will unzip them during OTA update.

ESP01 environment currently lacks support for OTA (it has it disabled) as the binary size is too large to support it (even with 2 step approach, which worked for 0.13). When flashing serially esptool compresses binary on the fly so there is no need for GZ files.
BTW the limiting size for OTA capability on ESP01 is about 726kB (with 128kB LittleFS partition). If the binary is less than that 2-step OTA is possible.

ESP02 is a funny freak. It will allow OTA because existing binary (in the flash) is not large enough to prohibit GZ upload but is large enough to prohibit un-gzipped upload (exactly the same process as with ESP01). As binary files grow, it is inevitable that at some point in the future even ESP02 won't be able to be updated OTA without 2-step process. Gzipped or not.

@w00000dy
Copy link
Contributor Author

What about nodemcu builds (esp8266 4MB)? Do we gain any advantage with bin.gz files here?

@blazoncek
Copy link
Collaborator

What about nodemcu builds (esp8266 4MB)? Do we gain any advantage with bin.gz files here?

A slightly faster OTA update, but otherwise none.

@w00000dy
Copy link
Contributor Author

Okay, so I don't think it's worth including the nodemcu bin.gz file in GitHub artifacts.
If you want it in artifacts let me know and I will change it.

@blazoncek blazoncek merged commit b7db5be into Aircoookie:0_15 Mar 29, 2024
17 checks passed
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.

3 participants