-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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 to SDK 1.5.4 #1377
Update to SDK 1.5.4 #1377
Conversation
Thanks a lot Yury!
Yes, that may be. If you want to change it and create a PR please look at #884 first. |
@marcelstoer updated it, CI green. |
Thanks Yury but you didn't actually fix #884, did you? AFAICS you updated the tool chain and compressed it with XZ instead of GZ. This shrinks the compressed file from 32MB to 20MB. However, the uncompressed content grew from 105MB to 112MB which indicates you didn't strip out libs and sources we don't need. @TerryE when you created #884 you claimed that
from which I conclude you did, at that time, have a list of stuff you removed from the tool chain? |
I don't really know which stuff can be stripped out without affection to build process. If you can prepare list of excluded files (or possible white-list?), I'll repack it. Also, I think this file used only on CI. Maybe move it outside repository and download dynamically from another source? |
@marcelstoer @TerryE what's new? |
@marcelstoer @djphoenix Yuri, I'll take a look today and post back. |
@djphoenix I actually suggested you create a separate PR for the SDK upgrade. |
If it would not break/corrupt build - OK, will cleanup this PR and we landed. |
@marcelstoer ready to land or not (then why?) |
Sorry working 12 hrs a day on the house at the moment -- we've got two crews in and I am dancing around trying to avoid stopping them. I know that I am being a blocker here, but I do suggest that this needs full review, if not by me then by one of the other committers :( Again -- Sorry Yuri. |
Terry, your comment primarily concerns the tool chain changes (#884), right? They're no longer part of this PR. As for the SDK upgrade I will verify what I can today and/or tomorrow. |
@djphoenix I did some testing tonight (wifi, node, gpio, net, http, mqtt, spi) - looks good apart from two tiny issues. You forgot to update the NodeMCU version in a couple of places:
|
OK, updated all references that I was found. |
Build success, bootup success, seems like all features keeps working.
Seems like CI using "esp-open-sdk.zip" from tools folder, maybe needed to update also.
Fixes #1144, #884.