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

Thoughts on RTOS_SDK options #38

Open
someburner opened this issue Sep 6, 2019 · 3 comments
Open

Thoughts on RTOS_SDK options #38

someburner opened this issue Sep 6, 2019 · 3 comments

Comments

@someburner
Copy link
Contributor

someburner commented Sep 6, 2019

Scanning over the RTOS SDK I saw a couple changes they've made to their lwip. Thoughts on these?

espressif/ESP8266_RTOS_SDK@d64f04b

espressif/ESP8266_RTOS_SDK@a183ccc

someburner@893a240

last one is on my repo, that's extracted from their kconfig defaults.

There are others too. Just wondering if you think it's worth following their changes for certain lwip things.

@d-a-v
Copy link
Owner

d-a-v commented Sep 6, 2019

Yes you can add these here too, or is this a feature request ?

Let me claim also here that espressif is again doing things not the best way with lwIP:
Instead of having a patch policy, they integrate their changes in their local copy. Once again they shoot themselves in the foot because it makes lwIP upgrade much more difficult (well, much less than with nonos-sdk though :)

@someburner
Copy link
Contributor Author

someburner commented Sep 6, 2019

If I can resolve the open-sdk PR for my project I can make these changes and submit another PR.

I agree they really did not think this through at all. Maintaining a set of patches is much cleaner and easier for other developers to follow. Trying to navigate through their source history is painful.

@someburner
Copy link
Contributor Author

Another one.. scanning over NONOS_SDK commits from last year.. espressif/ESP8266_NONOS_SDK@a9a39bb

I wish there was some sort of description for these regarding the set of circumstances leading them to make these changes.

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

No branches or pull requests

2 participants