Skip to content

Add MM_HALOW_ENABLE_LWIP to control lwIP integration. - #3

Merged
dpgeorge merged 2 commits into
micropython:mainfrom
dpgeorge:add-lwip-option
Sep 16, 2026
Merged

dpgeorge merged 2 commits into
micropython:mainfrom
dpgeorge:add-lwip-option

Conversation

@dpgeorge

Copy link
Copy Markdown
Member

It's disabled by default. That allows the tests to be a lot simpler because they don't rely on lwIP anymore.

@dpgeorge

Copy link
Copy Markdown
Member Author

@kwagyeman a small change, see what you think.

@kwagyeman

Copy link
Copy Markdown
Collaborator

From Claude:

Looks good — nice simplification, and defaulting MM_HALOW_ENABLE_LWIP off is
the right call.

One thing to confirm: is lwIP-off meant to be a buildable full-driver mode, or
just for the tests? The tests pass because they only compile
mm_halow_osal.c / mm_halow_sched.c, but mm_halow_ctrl.c still calls the
lwIP callbacks unconditionally (mm_halow_cb_tcpip_* /
mm_halow_cb_process_ethernet, around lines 134-151 and 442-462), and
mm_halow_lwip.c now compiles those out when lwIP is off — so a full driver
built with MM_HALOW_ENABLE_LWIP=0 would fail to link. If it's only for the
tests, all good; if it should build standalone, those call sites want the same
guard.

Signed-off-by: Damien George <damien@micropython.org>
No longer needed now that lwIP can be disabled in the driver.

Signed-off-by: Damien George <damien@micropython.org>
@dpgeorge

Copy link
Copy Markdown
Member Author

is lwIP-off meant to be a buildable full-driver mode

Yes, eventually. Following cyw43-driver, I'd like to make a test that has a mock SPI backend and tests at least calling mp_halow_init(...).

I've updated this PR so it can at least compile all src files with MM_HALOW_ENABLE_LWIP disabled.

A follow-up PR can add a mock test that builds the whole thing.

@dpgeorge
dpgeorge requested a review from kwagyeman September 16, 2026 01:59
@kwagyeman

Copy link
Copy Markdown
Collaborator

Okay, it's good. It's fine to merge. @dpgeorge

@kwagyeman kwagyeman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From claude:

Yes — approve it.

Why it's good to approve:

It does what it claims and CI is fully green (formatting, spelling, host×2, qemu×2).
My review point was addressed head-on. Damien consciously scoped this as incremental: all src files now compile with lwIP off, and he explicitly deferred the full standalone link (the mock-SPI test that "builds the whole thing") to a follow-up PR. That's a reasonable maintainer call, not an oversight.

@dpgeorge
dpgeorge merged commit b7dd5db into micropython:main Sep 16, 2026
6 checks passed
@dpgeorge
dpgeorge deleted the add-lwip-option branch September 16, 2026 02:06
@dpgeorge

Copy link
Copy Markdown
Member Author

Thanks!

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

Successfully merging this pull request may close these issues.

2 participants