Skip to content
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

Merged
merged 4 commits into from
Oct 10, 2024

Conversation

tannewt
Copy link
Member

@tannewt tannewt commented Oct 8, 2024

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.

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.
@tannewt tannewt requested a review from dhalbert October 8, 2024 22:06
@tannewt tannewt added this to the 9.2.0 milestone Oct 8, 2024
dhalbert
dhalbert previously approved these changes Oct 8, 2024
Copy link
Collaborator

@dhalbert dhalbert left a 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.

@tannewt
Copy link
Member Author

tannewt commented Oct 8, 2024

It should be fine in a beta too. I tested that MDNS still works.

@dhalbert
Copy link
Collaborator

dhalbert commented Oct 8, 2024

Merging and can be tested in the beta.

@anecdata
Copy link
Member

anecdata commented Oct 9, 2024

I'm getting the same hostname / mdns hostname for all four devices I set up: cpy-t_qtpy_esp32s2-3ffe2e90, despite unique MAC addresses.

(curl finds them all, but oddly only sometimes gets the redirect:
Location: http://cpy-t_qtpy_esp32s2-3ffe2e90.local/
other times it goes directly from circuitpython.local to the IP address --> huh, with a FQDN circuitpython.local. it does not redirect through the cpy name, with circuitpython.local it does redirect through the cpy` name ...saves a good bit of time to not have an additional redirect)

@RetiredWizard
Copy link

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.

@anecdata
Copy link
Member

anecdata commented Oct 9, 2024

Looks good, no mangling observed. mdns hostname correctly shows each MAC address. WW pages load with curl or browser by IP, mdns hostname, or circuitpython.local[.]

Links to other devices on the WW pages are correct.

@tannewt tannewt requested a review from dhalbert October 10, 2024 17:46
Copy link
Collaborator

@dhalbert dhalbert left a 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants