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

Only use internal URL when data is also using Wi-Fi #3015

Merged

Conversation

jpelgrom
Copy link
Member

Summary

Add a check to the external/internal URL logic to make sure that the device is not only connected to a Home Wi-Fi network, but is also using that network for data for using the internal URL. This better matches the expected behavior. Inspired by #3006.

A device can be connected to Wi-Fi but may be using a different (usually mobile) network for data for example if the internet is down or the network is considered "low quality". Expected log output in these situations is something like:

2022-10-29 15:32:21.317 11938-12081 UrlRepository           io....stant.companion.android.debug  D  localUrl is: true, usesInternalSsid is: true, usesWifi is: false
2022-10-29 15:32:21.318 11938-12081 UrlRepository           io....stant.companion.android.debug  D  Using external URL

Screenshots

n/a

Link to pull request in Documentation repository

n/a

Any other notes

Testing note: my Pixel on Android 13 simply hides the Wi-Fi icon from the status bar / shows the 4G data icon when Wi-Fi is not used for data, even though it may be connected. Check the system settings app to be sure.

Copy link
Member

@dshokouhi dshokouhi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement!

@marazmarci
Copy link
Contributor

Thanks for this improvement! It will surely be useful.

I was wondering about a less frequent use case, when the phone is connected to Ethernet and the device has cellular connection as a fallback. Here in this PR, the changes only take WiFi into account, so in the Ethernet use case, the same problem will arise.

@jpelgrom
Copy link
Member Author

I was wondering about a less frequent use case, when the phone is connected to Ethernet and the device has cellular connection as a fallback. Here in this PR, the changes only take WiFi into account, so in the Ethernet use case, the same problem will arise.

Currently ethernet won't be recognized as a home network, so I think this would be part of feature request #2532?

@marazmarci
Copy link
Contributor

Currently ethernet won't be recognized as a home network, so I think this would be part of feature request #2532?

Oh wow, yes, I saw this FR earlier, but forgot about it.

Yes, I also think this should be part of it.

@JBassett JBassett merged commit c57b80b into home-assistant:master Nov 3, 2022
@jpelgrom jpelgrom deleted the internal-check-active-connection branch November 3, 2022 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants