Skip to content

pkg/lwip: fix dependencies when LWIP IPv4 is used #21342

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
Merged
merged 2 commits into from
May 13, 2025
Merged

pkg/lwip: fix dependencies when LWIP IPv4 is used #21342

merged 2 commits into from
May 13, 2025

Conversation

krzysztof-cabaj
Copy link
Contributor

@krzysztof-cabaj krzysztof-cabaj commented Apr 1, 2025 •
edited
Loading

Contribution description

While working on PR #21333, when I by mistake enable LWIP IPv4, Murdock detected some compilation errors
associated with dependencies (see #21333 (comment), older versions of this automatic reply).

In file included from acd.c:71:
../../include/lwip/acd.h:97:48: error: 'struct etharp_hdr' declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
   97 | void acd_arp_reply(struct netif *netif, struct etharp_hdr *hdr);
      |                                                ^~~~~~~~~~
acd.c: In function 'acd_tmr':
acd.c:264:13: error: implicit declaration of function 'etharp_acd_probe' [-Werror=implicit-function-declaration]
  264 |             etharp_acd_probe(netif, &acd->ipaddr);
      |             ^~~~~~~~~~~~~~~~
acd.c:298:13: error: implicit declaration of function 'etharp_acd_announce' [-Werror=implicit-function-declaration]
  298 |             etharp_acd_announce(netif, &acd->ipaddr);
      |             ^~~~~~~~~~~~~~~~~~~
acd.c: At top level:
acd.c:376:43: error: 'struct etharp_hdr' declared inside parameter list will not be visible outside of this definition or declaration [-Werror]
  376 | acd_arp_reply(struct netif *netif, struct etharp_hdr *hdr)
      |                                           ^~~~~~~~~~

Similar error was observed and fixed for esp board in PR #21316.

This PR fixes this problem and code with LWIP_IPV4=1 passes all Murdock tests.

Testing procedure

Enable LWIP IPv4 in Makefile by setting LWIP_IPV4 ?= 1.

All Murdock compilation tests should pass.

Unfortunately, in current master some compilation tests fails.
What is interesting in my environment everything compile without problems (for example, for boards with cpu qn908x, samd5x or nrf52), so for further work and tests PR label CI: ready for build is needed.

Issues/PRs references

PR #21316
PR #21333

@github-actions github-actions bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports Area: examples Area: Example Applications labels Apr 1, 2025
@mguetschow mguetschow added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 2, 2025
@riot-ci
Copy link

riot-ci commented Apr 2, 2025 •
edited
Loading

Murdock results

✔️ PASSED

ae20e07 pkg/lwip: add missing dependencies

Success Failures Total Runtime
10341 0 10341 12m:26s

Artifacts

@github-actions github-actions bot added the Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms label Apr 2, 2025
@krzysztof-cabaj
Copy link
Contributor Author

@mguetschow thanks for CI: ready to build flag. I start working on fixing this issues.

@crasbe crasbe added the CI: no fast fail don't abort PR build after first error label Apr 2, 2025
benpicco
benpicco reviewed Apr 2, 2025
View reviewed changes
@krzysztof-cabaj krzysztof-cabaj requested a review from miri64 as a code owner April 2, 2025 16:14
@github-actions github-actions bot added Area: network Area: Networking Area: pkg Area: External package ports labels Apr 2, 2025
@krzysztof-cabaj krzysztof-cabaj force-pushed the pkg_lwip_ipv4_dependencies branch from 4124fb6 to 6917a73 Compare April 2, 2025 17:23
@github-actions github-actions bot added Area: doc Area: Documentation Area: boards Area: Board ports Area: Kconfig Area: Kconfig integration and removed Platform: ARM Platform: This PR/issue effects ARM-based platforms Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms Area: cpu Area: CPU/MCU ports labels Apr 2, 2025
@krzysztof-cabaj krzysztof-cabaj force-pushed the pkg_lwip_ipv4_dependencies branch 2 times, most recently from 3687efb to ba73128 Compare April 2, 2025 20:06
@github-actions github-actions bot removed the Area: doc Area: Documentation label Apr 2, 2025
@krzysztof-cabaj krzysztof-cabaj force-pushed the pkg_lwip_ipv4_dependencies branch 4 times, most recently from 52dd450 to 5054988 Compare April 25, 2025 11:46
@krzysztof-cabaj
Copy link
Contributor Author

I checked and with this PR all examples which allows LWIP IPv4 (nanocopa_server, gcoap, gcoap_dtls and paqo_mqtt) are green in Murdock.

Can we move this PR little forward?

@krzysztof-cabaj
Copy link
Contributor Author

@mguetschow, @crasbe, @benpicco, @miri64, @maribu ... sorry for broadcast ...

... but, can we move this PR little forward?

crasbe
crasbe reviewed May 6, 2025
View reviewed changes
@crasbe
Copy link
Contributor

crasbe commented May 6, 2025

I haven't used LWIP yet, so my review can't really assess the reason or logic behind the changes.

@krzysztof-cabaj krzysztof-cabaj force-pushed the pkg_lwip_ipv4_dependencies branch 2 times, most recently from 7b64bb7 to c287309 Compare May 6, 2025 12:01
@krzysztof-cabaj
Copy link
Contributor Author

krzysztof-cabaj commented May 6, 2025 •
edited
Loading

Hmmm ... is PR #21419 causing some other issue with LWIP?

I didn't check this with details, but after rebasing to most recent version I have some new errors and this PR name is suspicious ;)

What is interesting errors appear for LWIP both IPv4 and IPv6 - on my local machine.

PR #21467 fix this issue!
Murdock is green again.

@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels May 6, 2025
@krzysztof-cabaj krzysztof-cabaj mentioned this pull request May 6, 2025
Merged
@krzysztof-cabaj krzysztof-cabaj force-pushed the pkg_lwip_ipv4_dependencies branch 2 times, most recently from 03ab9c9 to 156334a Compare May 6, 2025 17:44
benpicco
benpicco reviewed May 6, 2025
View reviewed changes
@crasbe crasbe added the Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) label May 7, 2025
@krzysztof-cabaj krzysztof-cabaj force-pushed the pkg_lwip_ipv4_dependencies branch from 156334a to ae20e07 Compare May 13, 2025 09:34
@github-actions github-actions bot removed the Area: examples Area: Example Applications label May 13, 2025
benpicco
benpicco approved these changes May 13, 2025
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.

Thank you for the patch!

@benpicco benpicco enabled auto-merge May 13, 2025 11:48
@benpicco benpicco added this pull request to the merge queue May 13, 2025
Merged via the queue into RIOT-OS:master with commit cb7a327 May 13, 2025
27 checks passed
@krzysztof-cabaj
Copy link
Contributor Author

Dear All! Thanks for support during this PR and your valuable comments.

@krzysztof-cabaj krzysztof-cabaj mentioned this pull request May 31, 2025
Open
@Teufelchen1 Teufelchen1 added this to the Release 2025.07 milestone Jul 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@miri64 miri64 miri64 left review comments

@crasbe crasbe crasbe left review comments

@maribu maribu maribu left review comments

@benpicco benpicco benpicco approved these changes

@dylad dylad Awaiting requested review from dylad

@aabadie aabadie Awaiting requested review from aabadie

@kaspar030 kaspar030 Awaiting requested review from kaspar030

@bergzand bergzand Awaiting requested review from bergzand

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri

@jia200x jia200x Awaiting requested review from jia200x

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss

Assignees
No one assigned
Labels
Area: network Area: Networking Area: pkg Area: External package ports CI: no fast fail don't abort PR build after first error CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Milestone
Release 2025.07
Development

Successfully merging this pull request may close these issues.
8 participants
@krzysztof-cabaj @riot-ci @maribu @crasbe @miri64 @benpicco @Teufelchen1 @mguetschow