extmod/lwip-include: Factor common lwIP config into lwipopts_common.h. #16647
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #16647 +/- ##
=======================================
Coverage 98.58% 98.58%
=======================================
Files 167 167
Lines 21590 21590
=======================================
Hits 21285 21285
Misses 305 305 ☔ View full report in Codecov by Sentry. |
|
Code size report: |
projectgus
approved these changes
Jan 29, 2025
This lwIP configuration file has options that are common to all ports, and the ports are updated to use this file. This change is a no-op, the lwIP configuration remains the same for the four ports using this common file. This reduces code duplication, keeps the ports in sync, and makes it easier to update the configuration for all ports at once. Signed-off-by: Damien George <damien@micropython.org>
1071061 to
abb13b1
Compare
|
Is |
Probably not needed anymore. That was there originally as a full lwIP configuration for ports if they wanted to use it, but none ever did, the ports all needed their own custom configuration (there are also |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This lwIP configuration file has options that are common to all ports, and the ports are updated to use this file. This change is a no-op, the lwIP configuration remains the same for the four ports using this common file.
This reduces code duplication, keeps the ports in sync, and makes it easier to update the configuration for all ports at once.
Testing
For each port built a board with lwIP enabled:
RPI_PICO_W,PYBD_SF6,TEENSY41,ARDUINO_PORTENTA_C33. Built both before and after the commit in this PR. Then compared the binary firmware. The only things that changed in the binary were (1) the git commit hash in the version string; (2) the ordering of some of the pins in themachine.Pin.boarddict (for some reason the order of these are not deterministic). So I could conclude that the lwIP config changes are a no-op.Edit: also built an stm32 board with PPP enabled and it builds.