-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
ports/rp2: Enable PPP for Pico W. #16445
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
base: master
Are you sure you want to change the base?
Conversation
Code size report:
|
// Enable PPP networking. | ||
#define MICROPY_PY_NETWORK_PPP_LWIP (1) | ||
extern const struct _mp_obj_type_t mp_network_ppp_lwip_type; | ||
#define MICROPY_BOARD_NETWORK_INTERFACES { MP_ROM_QSTR(MP_QSTR_PPP), MP_ROM_PTR(&mp_network_ppp_lwip_type) }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to add this entry, it's done automatically in extmod/modnetwork.c
.
@@ -8,6 +8,11 @@ | |||
#define MICROPY_PY_NETWORK 1 | |||
#define MICROPY_PY_NETWORK_HOSTNAME_DEFAULT "PicoW" | |||
|
|||
// Enable PPP networking. | |||
#define MICROPY_PY_NETWORK_PPP_LWIP (1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could probably put this line in mpconfigport.h
as:
define MICROPY_PY_NETWORK_PPP_LWIP (MICROPY_PY_NETWORK)
Then it's available on all network-enabled boards, eg Pico 2 W.
Update to enable in Looks like there's a significant code size overhead, though! Edit: Looks like I need to limit to ports with LWIP enabled 🤦
Actually or maybe I have an order of operations thing going on in the header. Edit: I think this is the classic disagreement in patterns between:
|
Wait a minute, what's the diference between:
and
The latter seems to be set just for
|
mpconfigport.h: Default MICROPY_PY_NETWORK_PPP_LWIP to 1. Signed-off-by: Phil Howard <github@gadgetoid.com>
MICROPY_PY_LWIP_PPP is used in only one place and is generally set to the value of MICROPY_PY_NETWORK_PPP_LWIP. Remove the former. extmod/lwip-include/lwipopts_common.h: Replace MICROPY_PY_LWIP_PPP with MICROPY_PY_NETWORK_PPP_LWIP. ports/mimxrt/mpconfigport.h: Remove redundant MICROPY_PY_LWIP_PPP. ports/rp2/mpconfigport.h: Remove redundant MICROPY_PY_LWIP_PPP, default MICROPY_PY_NETWORK_PPP_LWIP to MICROPY_PY_LWIP. ports/stm32/mpconfigport.h: Remove redundant MICROPY_PY_LWIP_PPP. Signed-off-by: Phil Howard <github@gadgetoid.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #16445 +/- ##
=======================================
Coverage 98.38% 98.38%
=======================================
Files 171 171
Lines 22295 22295
=======================================
Hits 21936 21936
Misses 359 359 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Enables
MICROPY_PY_NETWORK_PPP_LWIP
for Pico W so that it can take advantage of PPP-supporting networking modules.This would probably be too onerous for a regular Pico build. Even though it could work, I suspect the networking stack will blow the tight firmware allocation.
Edit: Woof! That's quite a bit more code bloat than I'd expected!