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

Upgrade SDK to 3.0.4 #3284

Closed
wants to merge 1 commit into from
Closed

Conversation

marcelstoer
Copy link
Member

Fixes #3008

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/*.

@TerryE I'm not sure what to do about $RELEASE for latest-3.0. Isn't "latest 3.0" the same as 3.0.4 in the else-branch?

@TerryE
Copy link
Collaborator

TerryE commented Sep 21, 2020

@TerryE I'm not sure what to do about $RELEASE for latest-3.0. Isn't "latest 3.0" the same as 3.0.4 in the else-branch?

At the moment this is a bit of a mess. With this PR, the wget pulls archive/$(SDK_FILE_VER).zip with one of three file versions:

  • release/v3.0.0.zip (RELEASE=latest-3.0") This is a 3.0.0 release and we haven't been building against this for perhaps a year. IMO, this is obsolete and needs removed.
  • master.zip (RELEASE=master"). Note that master doesn't have the usage here as in our project. Master is actually more closely aligned to our dev as this is where all work-in-progress commits are made.
  • v3.0.4.zip (default). This is the latest 3.0 release (and closer to our release staging. This PR up this from an interim 3.0.1 commit to the tagged 3.0.4. Note that the 3.0.x release tags are labelled versions of master. (3.0.4 was a commit on 27 May 2020; the latest master commit was on 8 June.

Whatever we do this parameter should not be called RELEASE, but rather something like SDK_RELEASE. The default for this should be v3.0.4 and we define the SHA1 for this default. Any non-default should be as-is and the SHA1 should be "NA".

@marcelstoer
Copy link
Member Author

At the moment this is a bit of a mess.

Exactly 😞 Thanks for taking the time to describe the three options I noticed in the code but was too lazy to list.

I also wanted to dump the RELEASE=latest-3.0 branch. Good idea to rename RELEASE to SDK_RELEASE.

@nwf
Copy link
Member

nwf commented Sep 21, 2020

It looks like there are a lot of changes in Espressif's tree that should be applied to ours as well. This will require a lot more than just bumping some strings in the Makefile, I'm afraid; it looks rather like #2269 again. At a glace, it looks like at least these need to be ported over (I stopped reading the list when this got long enough; this is not even an exhaustive skim):

@marcelstoer
Copy link
Member Author

This will require a lot more than just bumping some strings in the Makefile

I thought so...remembering from the past that SDK upgrades (even at patch level) were major undertakings.

I had the constant nagging feeling that I didn't actually know what I was doing here with this PR 🙈 I can close it or leave it open so that whoever will do the job I'm not capable of can at least grab the first commit.

@nwf
Copy link
Member

nwf commented Sep 21, 2020

We are in a very frustrating position wrt Espressif, and it bums me out. I'll add doing the LwIP updates to my to-do list, but don't expect progress until this weekend at the earliest. FWIW, the Makefile changes do seem fine 👍.

@TerryE
Copy link
Collaborator

TerryE commented Sep 22, 2020

The nub of Nathaniel's comment is something that we've mentioned before: we forked LWiP, etc. a few years ago when Espressif first posted an open source stack as an alternative to the then closed version shipped with the SDK.

They subsequently switched to the OS version themselves and have done as range of bug fixes which our fork doesn't reflect.

I am not sure that we can easily revert back to the Espressif version, but the exercise that N proposes would be a good start. It's a few days work though.

@marcelstoer marcelstoer removed this from the Next release milestone Nov 3, 2020
@stale
Copy link

stale bot commented Apr 16, 2022

This issue 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 label Apr 16, 2022
@stale stale bot closed this Apr 30, 2022
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.

3 participants