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

Remove CONFIG_LWIP_LOCAL_HOSTNAME from S3 boards #9656

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

RetiredWizard
Copy link

As discussed in #9583 this removes the CONFIG_LWIP_LOCAL_HOSTNAME parameter from the sdkconfig files of all ESP32-S3 boards.

I actually just commented out the configuration setting but can easily update this to completely remove the lines if that is preferred.

@anecdata
Copy link
Member

For completeness: I checked an S2 board with an empty sdkconfig file, and it looks like boards without CONFIG_LWIP_LOCAL_HOSTNAME defined will have the hostname espressif, unless changed in user code (wifi.radio.hostname). The hostname shows up in the router via DHCP, and may be used by some other protocols.

@RetiredWizard
Copy link
Author

I submitted this not really knowing as much as I should have about core Circuitpython WiFi. It seems to me if the variable is used somewhere that is visible via Circuitpython we should probably leave it alone.

Feel free to re-open if there's a reason to pursue this.

I made these changes using a script so it would be relatively painless, if for some reason we wanted to expand to other chips or make a different change to the files/default values.

@anecdata
Copy link
Member

anecdata commented Sep 24, 2024

I didn't mean to shut this down, only highlight what the new hostname would be. We haven't been consistent with having CONFIG_LWIP_LOCAL_HOSTNAME or not, or how to name it. I don't know if there's a use case to rely on an expected default where the old expectation wouldn't be met... I'd be a little surprised, especially since multiple of the same board would look identical anyway. I'm perfectly OK with Scott's and Dan's assessment that it isn't needed.

@tannewt
Copy link
Member

tannewt commented Sep 24, 2024

I do want to get rid of these settings so that our IDF builds are more uniform across boards. I think we should set it to the board id by default though. That's basically what folks use this setting for anyway.

@tannewt
Copy link
Member

tannewt commented Sep 24, 2024

Perhaps we should unify this with the MDNS name too. It could be cpy-board_name-MAC.

@RetiredWizard RetiredWizard reopened this Sep 24, 2024
@RetiredWizard
Copy link
Author

Any pointers as to where the initialization should take place. It looks like common_hal_wifi_init happens when wifi is imported which sounds too late to me.

@anecdata
Copy link
Member

anecdata commented Sep 24, 2024

I would think common_hal_wifi_init is fine. There is no wifi Station or AP, or wifi network access, possible until that time, and user code can't access the hostname without importing wifi. It should happen before web workflow or auto-connect start up, but that's enforced I think by web workflow calling common_hal_wifi_init.

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

Successfully merging this pull request may close these issues.

3 participants