-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Fix ethernet component hostname handling #2010
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, the ethernet component would try to setup and connect before `esp_eth_enable` is called, which can be called right after `esp_eth_init` is called. The rest of the logic in `start_connect_`, for example setting up the hostname and dhcp information, should wait until after the ETH_START event is received. This would result in the first connection attempt always failing (as `esp_eth_enable` would only be called on the second run through `start_connect_`) and once connected, would fail to set the hostname correctly. This PR correctly runs `esp_eth_enable` and `esp_eth_init` in the `setup` for the component. This allows the ethernet component to connect on the first attempt, saving 15 sec (the connection timeout) on component startup. This PR adds some additional state tracking to the ethernet component to be able to know when the start event has been received and start the connection once the eth_start event is received, and also track the connection status through this state machine. This makes the state more explicit, rather than using `last_connected_` and `connected_` to track state. With this PR, because of the now correct setup, the hostname is correctly set. This could have been seen earlier by error checking the return from `tcpip_adapter_set_hostname`, as this would fail on the first call, before the esp_eth_enable. Error checking is added to the call to set the hostname, to catch further problems with this. Fixes esphome/issues#2184
OttoWinter
reviewed
Jul 13, 2021
Co-authored-by: Otto Winter <[email protected]>
Propagae previous change to all uses of the enum
jesserockz
reviewed
Jul 14, 2021
jesserockz
approved these changes
Jul 15, 2021
jesserockz
pushed a commit
that referenced
this pull request
Jul 18, 2021
Co-authored-by: Otto Winter <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this implement/fix?
Previously, the ethernet component would try to setup and connect before
esp_eth_enableis called, which can be called right afteresp_eth_initis called. The rest of the logic instart_connect_,for example setting up the hostname and dhcp information, should wait
until after the ETH_START event is received. This would result in the
first connection attempt always failing (as
esp_eth_enablewould onlybe called on the second run through
start_connect_) and onceconnected, would fail to set the hostname correctly.
This PR correctly runs
esp_eth_enableandesp_eth_initin thesetupfor the component. This allows the ethernet component toconnect on the first attempt, saving 15 sec (the connection timeout) on
component startup.
This PR adds some additional state tracking to the ethernet component to
be able to know when the start event has been received and start the
connection once the eth_start event is received, and also track the
connection status through this state machine. This makes the state more
explicit, rather than using
last_connected_andconnected_to trackstate.
With this PR, because of the now correct setup, the hostname is
correctly set. This could have been seen earlier by error checking the
return from
tcpip_adapter_set_hostname, as this would fail on thefirst call, before the esp_eth_enable. Error checking is added to the
call to set the hostname, to catch further problems with this.
Types of changes
Related issue or feature (if applicable):
Fixes esphome/issues#2184
Pull request in esphome-docs with documentation (if applicable): N/A
Test Environment
Example entry for
config.yaml:Checklist:
tests/folder).Ran test yaml on wesp32 device, saw that device would connect correctly
on the first attempt without requiring a re-try on the ethernet
connection, and once connected, had the correct hostname. Disconnected
and re-connected the ethernet cable to observe the state correctly
transition between connecting and connected.
If user exposed functionality or configuration variables are added/changed: