Add WearDns for handling DNS resolution on Wear devices

[yschimke]

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 https://github.com/home-assistant/android/issues/1866

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

[yschimke]

Open Questions

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

[home-assistant[bot]]

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

Learn more about our pull request process.

[TimoPtr]

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

[yschimke]

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]

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.

[TimoPtr]

@yschimke do you plan to continue this PR?

[yschimke]

Yep - apologies, slipped my mind. I'll try to update in the next 2 weeks.

[yschimke]

I'll add unit tests next. Also still need to test again on device after latest changes.

[yschimke]

@TimoPtr I forgot about your suggestion of no setting, and added one. Should I remove?

[TimoPtr]

@TimoPtr I forgot about your suggestion of no setting, and added one. Should I remove?

I'm fine with the settings on the watch. You'll need to add a screenshot in the PR description and also a link to the companion app documentation that adds a mention to this option in the troubleshooting section I would say.

[yschimke]

The setting doesn't work as you can't get to it before selecting a server. Removing.

[yschimke]

I'll follow up with unit tests tomorrow

[yschimke]

Spent too much time on what I thought would be a quick fix. If I find some time later, I'll pick it up and continue.

Thanks for the reviews so far.

[TimoPtr]

Spent too much time on what I thought would be a quick fix. If I find some time later, I'll pick it up and continue.

Thanks for the reviews so far.

You're very close to the finish line. If you are fine with it I can apply my last comments and merge it 👍🏻

[yschimke]

@TimoPtr go for it. Appreciated.

[TimoPtr]

@yschimke would you mind have a quick look to my changes?

[yschimke]

LGTM, thanks.

I'd suggest living with testing dnsRequest instead of onRequest because it keeps the tests much simpler.

[TimoPtr]

LGTM, thanks.

I'd suggest living with testing dnsRequest instead of onRequest because it keeps the tests much simpler.

Indeed, I didn't see during the review that it was a task from Google. I just tested the else branch

[dshokouhi]

So after this merge should we reach out to users impacted by #1866 and see if this helps?

[yschimke]

Thanks for the help landing this. it was working for me testing locally, but the real test will be on real networks. So any user feedback sooner can confirm or identify any follow up.