-
-
Notifications
You must be signed in to change notification settings - Fork 8.2k
ports/rp2: Fix rp2 mDNS issue via new cyw43 driver hooks. #17057
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
Conversation
Code size report:
|
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.
Thanks @mseminatore! This change looks good to me. I was able to test on a Pico2W using both STA and AP interfaces. STA worked fine, AP did not work for me but I think this is a problem with my host setup as a packet capture showed the mDNS responder on the Pico2W's AP was working fine.
As far as I can tell this driver is currently used only by the RP2 port.
This driver is also used on the stm32 port (PyBoard SF2 and SF6 plus multiple Arduino boards). However there's no common cyw43 config header for MicroPython yet (at least not outside the driver repo), so I think merging this PR and then expanding the fix to cover stm32 later is probably going to be the best path forward.
Thanks! The other PR was tagged with the RP2 port and for a release. @projectgus can you, or does someone else need to, move those designations over onto this PR? |
Have done. FWIW, for these kind of reasons it's almost always better to force-push to an existing PR branch rather than make a new PR. All good now, though. |
Thanks, yes, that was my intention. However, still recovering from an illness and managed to get the original PR into a bad state via misapplication of rebase and force-push. I wanted a clean PR so decided it was better to start over in this case. |
Thanks for getting back to this @mseminatore . I tested the following cases:
So it seems there's an issue with mDNS when in AP mode. Regardless of that, IMO we can merge this PR as-is because it's an obvious improvement. Investigating and fixing the AP mDNS could take some time. |
@projectgus @dpgeorge I have reproduced the issue with AP on a Pico W. I setup the Pico W as an AP and was unable to ping it from other local devices. I was able to connect to it, so the AP functionality is working. Looking over the code a bit, I was not able to see where/why the AP code path might not work. Is there something unique about netif construction for AP mode?
I'm interested in continuing to help refine things, so tend to agree that this PR adds value even if there it is not fully working in AP mode. |
If you want to try and debug it, you can use |
The rp2 port has an incomplete mDNS implementation. The code in `main.c` calls `mdns_resp_init()` which opens the UDP socket for mDNS. However, no code in the cyw43 driver makes the proper calls to `mdns_resp_add_netif()` and `mdns_resp_remove_netif()` to send the announce packets. The wiznet5k driver does make these calls and was used as a model for these changes. This commit attempts to address this by very small changes to the `ports/rp2/cyw43_configport.h` file. The change uses new cyw43 driver hooks to map the driver macros `CYW43_CB_TCPIP_INIT_EXTRA` and `CYW43_CB_TCPIP_DEINIT_EXTRA` to the appropriate lwIP mDNS calls. Fixes issue micropython#15297. Signed-off-by: Mark Seminatore <nebula_peeps4t@icloud.com>
dd1465e
to
f96417d
Compare
Merged. Thanks for the contribution @mseminatore ! Things to do in follow up work:
|
Would it be possible to post sample code to illustrate this working? (I'm sure it will be trivial, but I'm also fairly sure I've done everything required and it isn't working for me). As installed by Thonny, I'm using:
Code (in a REPL):
From a Pi with a wired connection to the same router (or a Mac with a wireless connection) I get variations on "Unknown host" when attempting to ping the Pico. |
@ChrisLawther can you please try your code without setting the hostname and let us know if that works? |
If I don't set the hostname I can see my router lists a device called That seems to be the initial DHCP interaction resulting in the DNS hostname being set, which I understood to be a completely different mechanism to mDNS. Maybe I need to do some more reading. Or maybe the resulting regular DNS behaviour is actually fine and I've just been distracted by the "magical" aspect of mDNS. |
@ChrisLawther I can't reproduce this myself, using the same setup (but no Thonny):
Variation of your sample code: import network, time
ssid = "myssid"
password = "mypassword"
hostname = "something"
network.hostname(hostname)
wlan = network.WLAN(network.STA_IF)
wlan.active(True)
wlan.connect(ssid, password)
while not wlan.isconnected():
time.sleep(1)
print("Connected!") I'm running this as "mpremote run pr17057.py" but that shouldn't make much difference. The main difference in the code is not setting On my Linux system:
Does mDNS work to lookup other hosts via
Yes, that is probably a separate thing where your router makes the DHCP hostname available via regular DNS (without the |
I did originally try both with and without the suffix. However, it is now working as expected so I must have been doing something subtly wrong. Thanks for confirming the correct omission(when setting) and inclusion (when pinging) of I did wonder whether it was my main machine "negative caching" the failure to look up the name when the pico was misconfigured, and continuing to report that failure even after applying a correct configuration, but I just tried that scenario and it's not that. A sequence of operations that does result in the apparent failure is:
A call to |
Summary
As a number of users have reported via issues and discussions, the RP2 port of MicroPython has an incomplete mDNS implementation. The code in main.c calls
mdns_resp_init()
which opens the UDP socket for mDNS. However, no code in the cyw43 driver of the RP2 port makes the proper calls tomdns_resp_add_netif()
andmdns_resp_remove_netif()
to send the announce packets. The wiznet5k driver does make these calls and was used as a model for these changes.This PR attempts to address this by very small changes to the
ports/rp2/cyw43_configport.h
file.The change uses new cyw43 driver hooks to map via driver macros
CYW43_CB_TCPIP_INIT_EXTRA
andCYW43_CB_TCPIP_DEINIT_EXTRA
to the appropriate LWIP MDNS calls.Testing
This was tested by building the firmware and deploying to a PicoW, starting a WLAN connection and pinging PicoW.local from several devices on the local network. I also tested changing the hostname from the default.
Trade-offs and Alternatives
Alternatively, if this PR is not accepted we should disable the partial implementation by setting
LWIP_MDNS_RESPONDER
to 0 inlwipopts.h
so that users can implement their own mDNS solutions without E_ADDRINUSE failures. A user should not have to build their own firmware to disable a partial/broken mDNS solution.