Skip to content

Conversation

@nwf
Copy link
Member

@nwf nwf commented Jun 16, 2018

Minimally build and boot tested, but the changes look pretty boring.
Fixes #2395

  • 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).

Minimally build and boot tested, but the changes look pretty boring.
Fixes nodemcu#2395
@marcelstoer marcelstoer added this to the next-release milestone Jun 17, 2018
@devsaurus
Copy link
Member

Looks good.

Espressif missed to update ESP_SDK_VERSION_PATCH to 1 in https://github.com/espressif/ESP8266_NONOS_SDK/blob/v2.2.1/include/version.h.
The firmware still reports

NodeMCU 2.2.0.0 build unspecified powered by Lua 5.1.4 on SDK 2.2.1(6ab97e9)

@nwf
Copy link
Member Author

nwf commented Jun 17, 2018

Bah humbug re: Espressif. We could leave it be or change user_version.h either to replace

#define NODE_VERSION_REVISION	ESP_SDK_VERSION_PATCH

with

#define NODE_VERSION_REVISION	1

or just bump NODE_VERSION_INTERNAL instead? I think I'd be more in favor of the last or the first, but this might just be bikeshedding.

@marcelstoer
Copy link
Member

marcelstoer commented Jun 17, 2018

We could leave it [...]

In light of the discussions in #2398 I don't think we should.

P.S. -> espressif/ESP8266_NONOS_SDK#136

@nwf
Copy link
Member Author

nwf commented Jun 17, 2018

@marcelstoer How do you feel about calling this 2.2.0.1 while we wait on Espressif?

@marcelstoer
Copy link
Member

@nwf I wouldn't do that. If Espressif fixes this we'd be shipping essentially the same SDK as before and, therefore, an change from 2.2.0.1 to 2.2.1.0 would feel odd to me.

@jmd13391
Copy link

Why don't we simply wait on SDK 2.2.1 until Espressif releases a fix?
Does anyone know if Espressif is aware of the issue?

@marcelstoer
Copy link
Member

marcelstoer commented Jun 18, 2018

SDK 2.2.1 is already released, that's what this PR is about. See my comment further up.

@nwf
Copy link
Member Author

nwf commented Jun 18, 2018

@marcelstoer Admittedly, I was imagining we'd wait until 2.2.2 or 2.3.0 or 3.0.0 or whatever to re-merge with upstream, so we'd not do a 2.2.0.1 to 2.2.1.0 transition. I'm content to wait a bit to see what Espressif does.

@marcelstoer
Copy link
Member

I'm content to wait a bit to see what Espressif does.

They haven't even commented on my PR in a month...

@marcelstoer marcelstoer merged commit fd745e0 into nodemcu:dev Jul 20, 2018
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.

4 participants