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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 2 commits into from
Dec 16, 2019
Merged

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

merged 2 commits into from
Dec 16, 2019

Conversation

gschorcht
Copy link
Contributor

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
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
@miri64
Copy link
Member

miri64 commented Dec 15, 2019

LGTM. :-)

benpicco
benpicco approved these changes Dec 15, 2019
View reviewed changes
Copy link
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
Contributor Author

@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
Contributor Author

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