Skip to content

pkg/lwip: overiddable settings in lwipopts.h#12948

Merged
benpicco merged 2 commits intoRIOT-OS:masterfrom
gschorcht:pkg/lwip/lwipopts_fix_cleanup
Dec 16, 2019
Merged

pkg/lwip: overiddable settings in lwipopts.h#12948
benpicco merged 2 commits intoRIOT-OS:masterfrom
gschorcht:pkg/lwip/lwipopts_fix_cleanup

Conversation

@gschorcht
Copy link
Copy Markdown
Contributor

@gschorcht gschorcht commented Dec 14, 2019

Contribution description

This PR provides the following changes:

  • TCPIP_THREAD_STACKSIZE is made overridable. On some platforms the default stack size is too small for lwIP's tcpip_thread so that the default stack size has to be overriden.
  • Platform dependent definition of TCPIP_THREAD_PRIO for ESP32 is removed. This is now defined by CFLAGS in ESP32's Makefile.

Testing procedure

Flash tests/lwip with the definition of TCPIP_THREAD_STACKSIZE for any ESP32 board:

USEMODULE='esp_wifi' CFLAGS='-DESP_WIFI_SSID=\"ssid\" -DESP_WIFI_PASS=\"pass\"' \
make BOARD=esp32-wroom-32 -C tests/lwip flash term

Use command ps. The stack size for thread tcpip_thread should be 3072 instead of the default stack size of 2048.

  6 | tcpip_thread         | bl mbox  _ |   2 |   3072 ( 2892) | 0x3ffb98cc | 0x3ffba260

Issues/PRs references

Figured out when testing PR #12903 for ESP32.

@gschorcht gschorcht added Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Dec 14, 2019
@gschorcht gschorcht requested a review from miri64 December 14, 2019 09:54
@gschorcht
Copy link
Copy Markdown
Contributor Author

gschorcht commented Dec 14, 2019 •
edited
Loading

@miri64 A review from you as the owner would be welcome. @benpicco or @MrKevinWeiss might be willing to test it.

@gschorcht gschorcht changed the title Pkg/lwip/lwipopts fix cleanup pkg/lwip: overiddable settings in lwipopts.h Dec 14, 2019
@gschorcht gschorcht mentioned this pull request Dec 14, 2019
Merged
benpicco
benpicco reviewed Dec 15, 2019
View reviewed changes
benpicco
benpicco reviewed Dec 15, 2019
View reviewed changes
@miri64
Copy link
Copy Markdown
Member

miri64 commented Dec 15, 2019

LGTM. :-)

benpicco
benpicco approved these changes Dec 15, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then please squash :)

@gschorcht
pkg/lwip: remove TCPIP_THREAD_PRIO for ESP32
aa3f132 
This definition is platform dependent and should be therefore done with CFLAGS in ESP332's Makefile.
@gschorcht gschorcht force-pushed the pkg/lwip/lwipopts_fix_cleanup branch from e88f644 to aa3f132 Compare December 15, 2019 22:51
@gschorcht
Copy link
Copy Markdown
Contributor Author

gschorcht commented Dec 16, 2019

@benpicco Squashed.

@benpicco benpicco merged commit 50f5060 into RIOT-OS:master Dec 16, 2019
@fjmolinas fjmolinas added this to the Release 2020.01 milestone Dec 16, 2019
@gschorcht gschorcht deleted the pkg/lwip/lwipopts_fix_cleanup branch December 16, 2019 10:05
@gschorcht
Copy link
Copy Markdown
Contributor Author

gschorcht commented Dec 16, 2019

Thanks for reviewing and merging

@gschorcht gschorcht mentioned this pull request Dec 16, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benpicco benpicco benpicco approved these changes

@miri64 miri64 Awaiting requested review from miri64 miri64 is a code owner

Assignees

No one assigned

Labels

Area: pkg Area: External package ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ESP Platform: This PR/issue effects ESP-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Milestone

Release 2020.01

Development

Successfully merging this pull request may close these issues.

4 participants

@gschorcht @miri64 @benpicco @fjmolinas