search

georgerobotics / cyw43-driver

Other
81 stars 42 forks source link

Make "netif_set_default" optional #59

Open kripton opened 1 year ago

kripton commented 1 year ago

I'm currently trying to port https://github.com/OpenLightingProject/rp2040-dmxsun to the Pico-W. of course with the plan to enable the WiFi interface as well. At the moment, lwIP is already in use with one network interface (TinyUSB's CDC NCM code to appear to the USB host as an USB ethernet interface). I already copied over pico-sdk's pico_cyw43_arch (https://github.com/kripton/rp2040-dmxsun/tree/4f3e76310f3861f8f7e384d4e3e6091997211c83/lib/pico_cyw43_arch_nolwip) since that unconditionally re-initialized the lwIP-stack (https://github.com/raspberrypi/pico-sdk/blob/5e9a5e827b3265fca53529883abdcc87608abad5/src/rp2_common/pico_cyw43_arch/cyw43_arch_poll.c#L59). Which is okay if the cyw43 is the only network interface on a board. However, even after that, when using the cyw43 as AP (easiest to test for now), the DHCP server on the USB ethernet-interface stopped working. If I comment out line https://github.com/georgerobotics/cyw43-driver/blob/83e0c700c252b392d33e70a5e9a241b921b166eb/src/cyw43_lwip.c#L206, the DHCP server works again. It would be nice if that line was guarded by an #ifdef so users of this driver can use it while having another netif as default.

I can provide a PR for that in the coming days.

peterharperuk commented 1 year ago

Which dhcp server has the problem? It sounds like an issue with the dhcp server implementation in that it's just using the default netif. I had to fix a similar issue recently https://github.com/bluekitchen/btstack/pull/448/commits/88cd67309cc94af545a96dac921d5ef21b3aa56e

All these dhcp servers look suspiciously similar.

kripton commented 1 year ago

Hi, indeed the code of the DHCP server I use is derived from that code base. It already had the detection, which netif the packet came in integrated but it missed the "send on the specific interface"-part. With your changes, the DHCP server works even if cyw43-driver sets the default interface. I can work with that, thanks for the explanations. I will now re-work the DHCP server so it can actually serve different IP ranges on different interfaces :)

So if you think setting the default interface in any case, just close this issue. In my opinion it wouldn't hurt making it optional, but I can still re-set another interface as default at the end of the init code ;)

UPDATE: Some typos which changed the meaning in the previous versions have been corrected

peterharperuk commented 1 year ago

it wouldn't hurt making it optional

True

dpgeorge commented 1 year ago

I agree these things (like setting the default netif) should be configurable. In fact we plan to rework the whole cyw43_cb_tcpip_init() function so that it can be completely replaced by the user if needed.

So for now please work around the issue by resetting the default netif after this cyw43 driver has started up.

peterharperuk commented 1 year ago

we plan to rework the whole cyw43_cb_tcpip_init() function so that it can be completely replaced

That would be good. It might be better if there was no lwip in the driver at all.