Add WearDns for handling DNS resolution on Wear devices #5170

yschimke · 2025-03-30T15:41:02Z

Summary

This commit introduces a new WearDns class to handle DNS resolution on Wear OS devices. It attempts to use the system DNS and falls back to mobile DNS if there's an issue.

The implementation includes:

mobile_network_helper: Constant for the mobile DNS capability.
/dnsLookup: Constant for the mobile DNS request.

Any other notes

I'm experimenting to address #1866

If this isn't working, then next option to explore is requiring Wifi. See https://google.github.io/horologist/network-awareness/

This commit introduces a new `WearDns` class to handle DNS resolution on Wear OS devices. It attempts to use the system DNS and falls back to mobile DNS if there's an issue. The implementation includes: - `dns_via_mobile`: Constant for the mobile DNS capability.

common/src/main/kotlin/io/homeassistant/companion/android/common/data/DataModule.kt

common/src/main/kotlin/io/homeassistant/companion/android/common/data/HomeAssistantApis.kt

wear/src/main/kotlin/io/homeassistant/companion/android/data/WearDataModule.kt

wear/src/main/kotlin/io/homeassistant/companion/android/data/WearDns.kt

This commit introduces a new `WearDns` class to handle DNS resolution on Wear OS devices. It attempts to use the system DNS and falls back to mobile DNS if there's an issue. The implementation includes: - `dns_via_mobile`: Constant for the mobile DNS capability.

# Conflicts: # wear/src/main/kotlin/io/homeassistant/companion/android/data/WearDns.kt

wear/src/main/kotlin/io/homeassistant/companion/android/phone/PhoneSettingsListener.kt

app/src/full/AndroidManifest.xml

yschimke · 2025-03-31T08:36:58Z

Open Questions

Should this be a user preference
Can we detect specific cases to retry (BT active, lan/local hostname)

app/src/full/kotlin/io/homeassistant/companion/android/data/wear/WearDataListener.kt

TimoPtr · 2025-03-31T09:18:01Z

app/src/full/kotlin/io/homeassistant/companion/android/data/wear/WearDataListener.kt

+import okhttp3.Dns
+
+@AndroidEntryPoint
+class WearDataListener : WearableListenerService() {


How do you envision this class to evolve? The name seems quite generic. Could you add a description within the code of the scope of the listener?

Yep, happy for guidance here.

I'll document the current scope (mobile providing helper APIs for networking/data).

In that case if you envision it to be for general networking/data I would suggest to follow Google documentation

For best performance, limit the number of listeners and use mutually exclusive filters where there are multiple listeners. A common pattern is to use prefix filters to listen to different namespaces within an application.

And introduce a namespace in the intent-filter something like android:pathPrefix="/network" and update the path to look like /network/dnsLookup

…ar/WearDataListener.kt Co-authored-by: Timothy <[email protected]>

TimoPtr

I think that for now we don't need a user preference for it (but it can be added later if needed). But I think it worse mentioning this behavior inside the https://companion.home-assistant.io/ (github https://github.com/home-assistant/companion.home-assistant).

For the retry I don't know we can go with your proposal and see if people are raising new issues about it.

app/src/full/kotlin/io/homeassistant/companion/android/data/wear/WearDataListener.kt

common/src/main/kotlin/io/homeassistant/companion/android/common/data/HomeAssistantApis.kt

TimoPtr · 2025-03-31T09:31:23Z

wear/src/main/kotlin/io/homeassistant/companion/android/data/WearDns.kt

+/**
+ * Wear specific implementation of Dns that defaults to the System DNS,
+ * but can fall back to relying on mobile Dns to resolve dns issues.
+ */


Be a bit more specific saying that it uses the messageAPI to query /dnsLookup with the hostname. Add a mention about the caching mechanism in place.

wear/src/main/kotlin/io/homeassistant/companion/android/data/WearDns.kt

TimoPtr · 2025-03-31T09:58:32Z

wear/src/main/kotlin/io/homeassistant/companion/android/data/WearDns.kt

+
+            return addresses
+        } catch (e2: Exception) {
+            e2.addSuppressed(e)


The two error combined (UnknownHostException) might be a bit hard to read. I think you better with proper logging here.
Add a log in the first catch something like that

Timber.e(e, "DNS resolution using the system DNS failed, fallback onto mobile DNS lookup."

TimoPtr · 2025-03-31T10:10:36Z

app/src/full/kotlin/io/homeassistant/companion/android/data/wear/WearDataListener.kt

+        if (path == "/dnsLookup") {
+            val hostname = String(request, Charsets.UTF_8)
+            return mainScope.async(Dispatchers.IO) {
+                Dns.SYSTEM.lookup(hostname).first().address


Why only returning the first? If I'm not wrong you could get at two InnetAddress one for ipv4 and one for ipv6. The wear could benefits having the two.

It avoid having some logic here you just forward whatever you received without transformation.

Yep. Should I json encode the request and response? Would be more inline with other uses?

This actually reminds me, we should prefer IPv4, as I'm not sure the BT proxied connection supports IPv6.

I'm ok with JSON. Even if we should prefer IPv4 let's not take any decision at this level. The wear should deal with that not the mobile.

TimoPtr · 2025-03-31T10:14:39Z

app/src/full/kotlin/io/homeassistant/companion/android/data/wear/WearDataListener.kt

+import okhttp3.Dns
+
+@AndroidEntryPoint
+class WearDataListener : WearableListenerService() {


In that case if you envision it to be for general networking/data I would suggest to follow Google documentation

For best performance, limit the number of listeners and use mutually exclusive filters where there are multiple listeners. A common pattern is to use prefix filters to listen to different namespaces within an application.

And introduce a namespace in the intent-filter something like android:pathPrefix="/network" and update the path to look like /network/dnsLookup

TimoPtr · 2025-03-31T10:18:36Z

wear/src/main/kotlin/io/homeassistant/companion/android/data/WearDns.kt

+    sealed interface CacheResult {
+        val storedAt: Instant
+    }
+
+    data class NegativeCacheHit(val exception: Exception, override val storedAt: Instant) : CacheResult
+    data class PositiveCacheHit(val value: List<InetAddress>, override val storedAt: Instant) : CacheResult


Can you make this private?

TimoPtr · 2025-03-31T10:21:54Z

wear/src/main/kotlin/io/homeassistant/companion/android/data/WearDns.kt

+        if (response.isEmpty()) {
+            throw UnknownHostException("Mobile helper unable to resolve $hostname")
+        }


I'm not sure we should do that, the normal behavior of OkHttp is just passing the empty list down the chain. If you received an empty list for the API forward it OkHttp will fail later.

Since we inject this class in the HomeAssistantAPI I rather replicate exactly what the default behavior does.

All current implementations of Dns throw on empty.

Dns.SYSTEM - https://github.com/square/okhttp/blob/ec41b8b77834849f6d3bd84302f8c52cb264f7ad/okhttp/src/commonJvmAndroid/kotlin/okhttp3/Dns.kt#L50

DnsOverHttps - https://github.com/square/okhttp/blob/ec41b8b77834849f6d3bd84302f8c52cb264f7ad/okhttp-dnsoverhttps/src/main/kotlin/okhttp3/dnsoverhttps/DnsOverHttps.kt#L154

As you say it ends up the same, butI think throwing here is more correct.

wear/src/main/kotlin/io/homeassistant/companion/android/data/WearDns.kt

home-assistant · 2025-03-31T10:30:28Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

…on/data/HomeAssistantApis.kt Co-authored-by: Timothy <[email protected]>

…earDns.kt Co-authored-by: Timothy <[email protected]>

TimoPtr · 2025-03-31T10:49:58Z

Would you mind adding some Unit test? I've added recently all the necessary tools to add them.

yschimke · 2025-04-01T06:33:42Z

Would you mind adding some Unit test? I've added recently all the necessary tools to add them.

@TimoPtr testing is hard with the DataLayer. In either instrumentation (need paired devices), or unit test (robolectric with custom shadows?).

I'll refactor to have some API we control for the DataLayer parts and test the rest of the structure.

I'm assuming unit tests based on this being recently added https://github.com/home-assistant/android/blob/master/common/src/test/kotlin/io/homeassistant/companion/android/common/data/websocket/impl/entities/ConversationResponseTest.kt

And put them under wear/src/test/kotlin

TimoPtr · 2025-04-01T08:53:57Z

Would you mind adding some Unit test? I've added recently all the necessary tools to add them.

@TimoPtr testing is hard with the DataLayer. In either instrumentation (need paired devices), or unit test (robolectric with custom shadows?).

I'll refactor to have some API we control for the DataLayer parts and test the rest of the structure.

I'm assuming unit tests based on this being recently added https://github.com/home-assistant/android/blob/master/common/src/test/kotlin/io/homeassistant/companion/android/common/data/websocket/impl/entities/ConversationResponseTest.kt

And put them under wear/src/test/kotlin

Yes unit test, unit test with robolectric and instrumentation test have been setup recently. For now it only contains very simple tests, I used them to work on the CI that run everything now. Lemme know if you need anything for writing the tests. I can help since it's still WIP.

home-assistant bot added the cla-signed label Mar 30, 2025

github-advanced-security bot found potential problems Mar 30, 2025

View reviewed changes

yschimke added 3 commits March 30, 2025 17:30

Merge remote-tracking branch 'yschimke/wear_dns' into wear_dns

221fe3b

# Conflicts: # wear/src/main/kotlin/io/homeassistant/companion/android/data/WearDns.kt

Fixes and move code around

d2fdecf

github-advanced-security bot found potential problems Mar 30, 2025

View reviewed changes

app/src/full/AndroidManifest.xml Fixed Show fixed Hide fixed

yschimke added 3 commits March 30, 2025 21:14

Cleanup

21c9abc

Add caching

aaec58f

Cleanup

86e74fc

yschimke marked this pull request as ready for review March 31, 2025 05:38

TimoPtr reviewed Mar 31, 2025

View reviewed changes

app/src/full/kotlin/io/homeassistant/companion/android/data/wear/WearDataListener.kt Outdated Show resolved Hide resolved

TimoPtr reviewed Mar 31, 2025

View reviewed changes

Update app/src/full/kotlin/io/homeassistant/companion/android/data/we…

c6e1b82

…ar/WearDataListener.kt Co-authored-by: Timothy <[email protected]>

TimoPtr requested changes Mar 31, 2025

View reviewed changes

home-assistant bot marked this pull request as draft March 31, 2025 10:30

yschimke and others added 2 commits March 31, 2025 11:44

Update common/src/main/kotlin/io/homeassistant/companion/android/comm…

9a0c2a0

…on/data/HomeAssistantApis.kt Co-authored-by: Timothy <[email protected]>

Update wear/src/main/kotlin/io/homeassistant/companion/android/data/W…

b42f737

…earDns.kt Co-authored-by: Timothy <[email protected]>

jpelgrom added the Wear OS label Apr 15, 2025

Uh oh!

Add WearDns for handling DNS resolution on Wear devices #5170

Are you sure you want to change the base?

Add WearDns for handling DNS resolution on Wear devices #5170

Uh oh!

Conversation

yschimke commented Mar 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Any other notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yschimke commented Mar 31, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TimoPtr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

home-assistant bot commented Mar 31, 2025

Uh oh!

TimoPtr commented Mar 31, 2025

Uh oh!

yschimke commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TimoPtr commented Apr 1, 2025

Uh oh!

Uh oh!

yschimke commented Mar 30, 2025 •

edited

Loading

yschimke commented Apr 1, 2025 •

edited

Loading