-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
MDNS hostname match DHCP. Fix collision mangling #9699
Conversation
MDNS defaults to the hostname used by DHCP/LWIP since it is now unique. This makes it the same either way. This also updates esp-protocols (used for mdns) with a patch to ensure that mangled names are the ones that collide. (circuitpython.local collides but was causing cpy- to be mangled.) Fixes micropython#6869 again.
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.
This looks fine to me but we can await testing by @anecdata and @RetiredWizard as well.
It should be fine in a beta too. I tested that MDNS still works. |
Merging and can be tested in the beta. |
I'm getting the same hostname / mdns hostname for all four devices I set up: ( |
Tested on BPI PIcow S3 and Hacktablet, the Web workflow page and my DHCP server both show the expected hostname with the correct 😊 MAC address. |
Looks good, no mangling observed. mdns hostname correctly shows each MAC address. WW pages load with curl or browser by IP, mdns hostname, or Links to other devices on the WW pages are correct. |
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 @anecdata and @RetiredWizard for testing!
There is some build problem with the latex pdf doc build. I am going to ignore that for now and debug it separately. I will go ahead and merge this.
MDNS defaults to the hostname used by DHCP/LWIP since it is now unique. This makes it the same either way.
This also updates esp-protocols (used for mdns) with a patch to ensure that mangled names are the ones that collide. (circuitpython.local collides but was causing cpy- to be mangled.)
Fixes #6869 again.
@RetiredWizard this changes the hostname slightly because it hit an assert when over 32 characters. cpy, board and mac are now dash separated too. That way the board's _ don't confusing things.
@anecdata Please test this for the MDNS collision mangling. I just checked that MDNS still works.