Uses outdated lwIP API?

[lurch]

Related to raspberrypi/pico-examples#247

Whilst I wasn't able to reproduce the originally-reported problem in that issue, curiosity prompted me to do a bit of digging anyway... 😉

https://github.com/georgerobotics/cyw43-driver/blob/main/src/cyw43_lwip.c#L182 does

mdns_resp_add_netif(n, mdns_hostname, 60);

however the third parameter to mdns_resp_add_netif was removed from lwIP on 13th November 2018. Looking at the dates of the lwIP release-tags, I guess this means that there was a backwards-incompatible API change between the 2.1.1 and 2.1.2 versions of lwIP 😕 And annoyingly, even though https://savannah.nongnu.org/projects/lwip mentions the 2.1.3 release of lwIP (which agrees with GitHub), http://www.nongnu.org/lwip/ shows the Doxygen for the 2.1.0 release, which uses the old API 🤦‍♂️

[dpgeorge]

In MicroPython we use the STABLE-2_1_2_RELEASE of lwIP. The signature of the MDNS function in that release is:

err_t mdns_resp_add_netif(struct netif *netif, const char *hostname, u32_t dns_ttl);

So it looks correct for that version.

I guess the point is, we should update to STABLE-2_1_3_RELEASE 😄

[lurch]

In MicroPython we use the STABLE-2_1_2_RELEASE of lwIP.

Well that's kinda awkward, as pico-sdk uses lwip @ 239918c which appears to be the current HEAD of lwIP master.
ping @kilograham and @peterharperuk

Does the code in cyw43-driver need to be conditional on what version of lwIP it's being compiled against?

Looks like lwIP's STABLE-2_1_2_RELEASE is dated 22 Nov 2018, STABLE-2_1_3_RELEASE is dated 10 Nov 2021 and master is dated 9 May 2022.

Oh! Looks like STABLE-2_1_3_RELEASE also has the function-signature mdns_resp_add_netif(struct netif *netif, const char *hostname, u32_t dns_ttl), so I was clearly wrong in my earlier complaint that lwIP were breaking APis in minor version releases, oops 🤦‍♂️

EDIT: Ah, and STABLE-2_1_x (dated 10 Nov 2021) also uses the three-arg form of mdns_resp_add_netif, but is "120 commits ahead, 548 commits behind master" 😭

[dpgeorge]

Does the code in cyw43-driver need to be conditional on what version of lwIP it's being compiled against?

Yes it probably does. This driver can support various lwIP versions if needed, and to do that would use #if conditions.

But right now I think everything works with STABLE-2_1_2_RELEASE and current HEAD of lwIP master branch?

[lurch]

But right now I think everything works with STABLE-2_1_2_RELEASE and current HEAD of lwIP master branch?

Yes, I believe so (the other people I pinged will know more about this than me). But I guess raspberrypi/pico-examples#247 implies that things might break if different preprocessor-definitions are used? 🤷

[dpgeorge]

See micropython/micropython#8960 for update of MicroPython to 2.1.3.

[peterharperuk]

Is it really worth our time to get the driver to work with an old version of lwip? It seems to work with 2.1.2, 2.1.3 and master?

[lurch]

Yeah, apologies for any confusion my misunderstanding caused - all the 2.1.x versions of lwIP are API-compatible; so it's only the 2.1.x versions and master that have slight API-breakage under specific circumstances. STABLE-2_1_3_RELEASE (as used by MicroPython) is the latest tagged release of lwIP and dates from 10 Nov 2021 whereas lwIP master (as used by pico-sdk) currently dates from 9 May 2022.

I'm happy for this to be closed if everyone is happy with the current situation?

[peterharperuk]

Actually, I can repro it now on the sdk.