core
core copied to clipboard
Android TV Remote integration
Proposed change
Implement Android TV Remote integration that allows sending commands and launching apps on an Android TV. This contrary to the existing Android TV integration doesn't require ADB and enabling developer tools and it's much faster in executing commands. In addition it supports zeroconf discovery. The only step needed is for the user to enter the PIN shown on the Android TV once they initiate pairing.
Discovery:
Pairing:
Once installed you can add buttons in lovelace to execute commands:
I marked it platinum from the start since I think it will make it easier to justify it during the review. Test coverage is 100%. For reference the requirements are below where I checked all of them except the non applicable ones:
Silver 🥈
This integration is able to cope when things go wrong. It will not print any exceptions nor will it fill the log with retry attempts.
- [x] Satisfying all No score level requirements.
- [x] Connection/configuration is handled via a component.
- [ ] ~~Set an appropriate
SCAN_INTERVAL
(if a polling integration)~~ - [ ] ~~Raise
PlatformNotReady
if unable to connect during platform setup (if appropriate)~~ - [x] Handles expiration of auth credentials. Refresh if possible or print correct error and fail setup. If based on a config entry, should trigger a new config entry flow to re-authorize. (docs)
- [x] Handles internet unavailable. Log a warning once when unavailable, log once when reconnected.
- [x] Handles device/service unavailable. Log a warning once when unavailable, log once when reconnected.
- [x] Operations like service calls and entity methods (e.g. Set HVAC Mode) have proper exception handling. Raise
ValueError
on invalid user input and raiseHomeAssistantError
for other failures such as a problem communicating with a device. - [x] Set
available
property toFalse
if appropriate (docs) - [x] Entities have unique ID (if available) (docs)
Gold 🥇
This is a solid integration that is able to survive poor conditions and can be configured via the user interface.
- [x] Satisfying all Silver level requirements.
- [x] Configurable via config entries.
- [x] Don't allow configuring already configured device/service (example: no 2 entries for same hub)
- [x] Discoverable (if available)
- [x] Set unique ID in config flow (if available)
- [x] Raise
ConfigEntryNotReady
if unable to connect during entry setup (if appropriate)
- [x] Entities have device info (if available) (docs)
- [x] Tests
- [x] Full test coverage for the config flow
- [x] Above average test coverage for all integration modules
- [x] Tests for fetching data from the integration and controlling it (docs)
- [x] Has a code owner (docs)
- [ ] ~~Entities only subscribe to updates inside
async_added_to_hass
and unsubscribe insideasync_will_remove_from_hass
(docs)~~ - [ ] ~~Entities have correct device classes where appropriate (docs)~~
- [x] Supports entities being disabled ~~and leverages
Entity.entity_registry_enabled_default
to disable less popular entities (docs)~~ - [ ] ~~If the device/service API can remove entities, the integration should make sure to clean up the entity and device registry.~~
- [x] When communicating with a device or service, the integration implements the diagnostics platform which redacts sensitive information.
Platinum 🏆
Best of the best. The integration is completely async, meaning it's super fast. Integrations that reach platinum level will require approval by the code owner for each PR.
- [x] Satisfying all Gold level requirements.
- [x] Set appropriate
PARALLEL_UPDATES
constant - [x] Support config entry unloading (called when config entry is removed)
- [x] Integration + dependency are async (docs)
- [ ] ~~Uses aiohttp or httpx and allows passing in websession (if making HTTP requests)~~
- [x] Handles expired credentials (if appropriate)
Type of change
- [ ] Dependency upgrade
- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] New integration (thank you!)
- [ ] New feature (which adds functionality to an existing integration)
- [ ] Deprecation (breaking change to happen in the future)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests
Additional information
- This PR fixes or closes issue: fixes #
- This PR is related to issue:
- Link to documentation pull request: https://github.com/home-assistant/home-assistant.io/pull/26659
Checklist
- [x] The code change is tested and works locally.
- [x] Local tests pass. Your PR cannot be merged unless tests pass
- [x] There is no commented out code in this PR.
- [x] I have followed the development checklist
- [x] I have followed the perfect PR recommendations
- [x] The code has been formatted using Black (
black --fast homeassistant tests
) - [x] Tests have been added to verify that the new code works.
If user exposed functionality or configuration variables are added/changed:
- [x] Documentation added/updated for www.home-assistant.io
If the code communicates with devices, web services, or third-party tools:
- [x] The manifest file has all fields filled out correctly.
Updated and included derived files by running:python3 -m script.hassfest
. - [x] New or updated dependencies have been added to
requirements_all.txt
.
Updated by runningpython3 -m script.gen_requirements_all
. - [ ] For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
- [ ] Untested files have been added to
.coveragerc
.
To help with the load of incoming pull requests:
- [ ] I have reviewed two other open pull requests in this repository.
Just a few pieces of feedback
That being said - You may want to decrease the size of this PR if you want maintainers to review it.
https://developers.home-assistant.io/docs/review-process#creating-the-perfect-pr
Potentially remove zeroconf and anything else you can think of that isn't necessary for a initial release.
Secondly, I'm not sure what the maintainers will think about adding another integration for android tv. Is there no chance of you just taking the improvements you have and applying it to the already existing android tv - I'm sure that is what maintainers are going to prefer.
Yes I'm aware of the preference on small PRs. I prefer them myself at work too. I considered extracting the diagnostics in a separate PR but that is essentially just 5 lines. I didn't consider separating zeroconf since it's an important feature of this integration. If I do it I could maybe reduce it by 20 lines or so of non testing code.
I prefer keeping this as a separate integration. Code would be too complicated having the existing integration support both for no added benefit that I can see. In addition in the past I was told to create a new integration in https://github.com/home-assistant/core/pull/82188 I believe the same applies here since it's using a different api.
Great work 👍. Tested, everything works. This could potentially be a good alternative to the current Android TV integration. So I would like to suggest a few things:
- Remove the activity functionality in the remote for managing applications and add a full-fledged media player platform in a future PR that can switch applications by selecting source or by
media_player.play_media
service call. - Rename the integration without reference to the
remote
, for example toGoogle TV
. To tell the truth, I always thought that the current Android TV integration should be namedAndroid ADB
since, firstly, it works not only with Android TV devices, but also with any Android devices, including mobile phones/tablets/wall panels, and secondly, it does not use protocols associated with Android TV exclusively, as this integration does.
Great work 👍. Tested, everything works. This could potentially be a good alternative to the current Android TV integration. So I would like to suggest a few things:
- Remove the activity functionality in the remote for managing applications and add a full-fledged media player platform in a future PR that can switch applications by selecting source or by
media_player.play_media
service call.- Rename the integration without using without reference to the
remote
, for example toGoogle TV
. To tell the truth, I always thought that the current Android TV integration should be namedAndroid ADB
since, firstly, it works not only with Android TV devices, but also with any Android devices, including mobile phones, and secondly, it does not use protocols associated with Android TV exclusively, as this integration does.
Thanks for testing and confirming it works for you. What devices did you test it with? I have tested it with Nvidia Shield TV, Chromecast with Google TV and a TV with integrated Google TV.
- I don't have any plans for adding the media player platform since the cast integration already works well for my Android TV devices with more features than this could ever provide. For example the Android TV remote protocol doesn't provide playback status. Maybe the cast integration doesn't work for Fire TV devices? In that case I can see the benefit supporting the media platform entity here, if Fire TV devices support this protocol, but there should be an option to disable it.
- I agree the existing integration should have been named Android ADB. I did consider naming this integration Google TV but that's the name of the new interface. This integration should work with Android TV devices without the new Google TV interface.
Thanks for testing and confirming it works for you. What devices did you test it with? I have tested it with Nvidia Shield TV, Chromecast with Google TV and a TV with integrated Google TV.
Works well with my Sony Bravia 55XH9096 on Android TV 10 and XGIMI Elfin Projector on Android TV 11.
I don't have any plans for adding the media player platform since the cast integration already works well for my Android TV devices with more features than this could ever provide. For example the Android TV remote protocol doesn't provide playback status. Maybe the cast integration doesn't work for Fire TV devices? In that case I can see the benefit supporting the media platform entity here, if Fire TV devices support this protocol, but there should be an option to disable it.
The media players may not have a playback state, and can be labeled as assumed_state
. In this case, all playback control buttons will be available in UI. See #84885 as example.
I think that if this integration will offer a media player, then they will be perfectly combined with the Cast integration through the universal integration. Not all applications return MediaSession
state and in this case the Cast integration does not show the status of the device at all. In addition, Cast integration cannot turn on devices from the off
state and cannot launch apps.
I agree the existing integration should have been named Android ADB. I did consider naming this integration Google TV but that's the name of the new interface. This integration should work with Android TV devices without the new Google TV interface.
Of course ideally if this integration is called Google TV
. Then rename the old Android TV to Android ADB, and then add an Android TV virtual integration that references that. But there will be too much bureaucracy, and it requires a thorough involvement of maintainers in this matter.
Implement Android TV Remote integration that allows sending commands and launching apps on an Android TV. This contrary to the existing Android TV integration doesn't require ADB and enabling developer tools and it's much faster in executing commands. In addition it supports zeroconf discovery. The only step needed is for the user to enter the PIN shown on the Android TV once they initiate pairing.
So why not implement that in the existing android tv integration? Adding another integration that does the same sounds super confusing to an end-user? To me that sounds like a great optimization of the existing integration?
I prefer keeping this as a separate integration.
I'm not sure if I agree. It is making code simpler at the cost of the user experience.
I don't see a clean way adding this functionally in the existing integration that will be easy to maintain. They are using different APIs. The existing one with ADB is more flexible and powerful but harder to setup for average users. This is using an API limited to remote support, thus the name. Having both implementations in the same integration might actually cause user confusion since they will get different functionality based on whether they have done the extra step to enable developer tools or the extra step of pairing with PIN. Thinking about it the config flow will be a mess to support both.
@balloob in the past, see https://github.com/home-assistant/core/pull/82188 when I wanted to add the ability to send commands to Google Assistant in the existing integration to avoid user confusion with similarly named integrations, said to create a new integration since it's a different API and not let frontend concerns drive backend decisions. I believe the same applies here.
Per @balloob 's comment in https://github.com/home-assistant/core/pull/82188 that we have brands to group integrations in the frontend, we could put both integrations under Android TV brand and maybe rename the existing one to Android TV ADB.
Per @balloob 's comment in https://github.com/home-assistant/core/pull/82188 that we have brands to group integrations in the frontend
Yes, that is to make things easier when different type of devices are used, however, that is not the case here. This is adding the exact same functionality again.
Brands help a lot. However, a user still has to be able to make a choice. I think this will worsen it a lot. I don't see how this is helping with any confusion.
I don't care if it is backend or frontend driven. The UX counts, at any point in time (no matter the origin).
@Drafteed suggested earlier to rename the existing one to Android ADB. Would that help?
Can you clarify how having two integrations would be confusing to the user? Most users will likely just install this since it supports discovery and no advanced extra steps such as enabling developer tools. I also assume most users are interested in remote control, powering on/off, maybe launching apps and not media player support since the cast integration already adds such functionally for Android TV devices. So regulars users don't even need to know the existing integration exists. Advanced users that want more functionally (e.g. screen capture, sending any adb command) can install the existing integration.
Having just one integration might be more confusing to regular users since they won't know how to enable developer tools to get the extra functionality and they might come across a post or video with the extra functionality and they will be wondering why they don't have it when they already have the integration installed. Having separate integrations I think makes the distinction more clear. The new one only gives TV remote functionally, the existing one gives the same TV remote functionality (using a different implementation) among much more.
To your point about having two different integrations having similar functionally, the Google Cast and Android TV integrations both add a media player entity for Android TV devices.
@frenck The current integration of Android TV is not really related directly to Android TV. The integration simply polls the debug interface and can work with any Android device such as phones, tablets or wall panels. Some users use this integration with TV Boxes on native Android (not Android TV).
In turn, this integration works exclusively with Android TV devices, as it interacts with a special native service called Android TV Remote Service, which exists only in real Android TV devices. Moreover, this integration uses local_push
connection class, unlike the existing one. So I don't think it's wise to add this to the old integration, as it will further complicate or break an already slow and complicated integration.
At the same time, as I wrote above, I think the old integration should have been called Android ADB
, since it uses hacky connection methods, and not "official ones", and can works with any Android device via ADB. But already this integration could rightfully be called Android TV
or Google TV
, and I think that it would be possible to make a worthy alternative to the slow old integration out of it.
A very good point in your comment, that I forgot, is that the the two integrations are of different connection class: local push vs local polling. If we were to combine them, what connection class would it be?
I'm not commenting here as a user who just started with integrating Google Cast / Android TV into my System AMD trying different "remote Control" solutions available in hacs at the Moment.
Honestly speaking - I don't Care If this will be integrated into the existing Integration, or will be available as another one... as Long, as it would improve the ux at all.
My current concern is, that my Android TV device seems to have issues with the Android TV Integration at all - ist dies Not Show Playback Information & it does Not Switch between Apps / sources. So the current Integration is pretty useless ATM.
The Cast seems to provide those Information - at least, for most - but not for all Apps. So the Cast solution is not working for Automations and device Control in all cases.... Despite of Starting the device and selecting the App.
In Addition to that: It seems, that Cast is loosing the Connection after playing Media for some time... Then, it does no longer show the Playback Information and my Automations are also not working anymore... The same applies to the current remote Controllers available on hacs... After some time I cannot Control the device anymore.
So... Finally: I don't Care If this will be included into the existing Integration or Not. I am looking Forward to a solution that improve the current Situation.
In Terms of different Communication Methods, APIs I so agree with the Idea to have it as another / separate Integration. But: I would also Like to See the "Media Player" component integrated into this... I really Like the "light Design" of the Media Player Card/component...
Let's rename the existing androidtv
integration to “Android debug bridge” in the manifest.json and have this integration be called Android TV Remote
Sounds good. Doing that in https://github.com/home-assistant/core/pull/90657 and https://github.com/home-assistant/home-assistant.io/pull/26830
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks :+1:
Thanks for the review and apologies for the PR size. Given this is using discovery and a lot of users have Android TV devices I tried my best to follow best practices and aimed for platinum quality from the beginning.
@tronikos I have found that when the Android TV device is physically turned off, the entity in the HA remains on. Moreover, it looks like the connection is not being closed/recreated, and if the device comes back online, the next command will be ignored (as the reconnect process starts).
Do you have more than one Android TV device? Is this specific to a particular device? The three different devices I have all report off as soon as I physically turn them off. Maybe some devices don't really turn off or don't report they are off?
This behavior is observed on all my three devices:
- Sony Bravia 55XH9096 on Android TV 10
- XGIMI Elfin Projector on Android TV 11.
- Xiaomi Mi TV Stick 4K on Android TV 11.
Maybe some devices don't really turn off or don't report they are off?
I physically unplugged them from power.
Thanks Paulus! Thanks @Drafteed for reporting the issue. I was able to reproduce it with this integration but not with the demo.py included in my library which is strange since they both use the same callback mechanism to get notified when the device becomes unavailable. I'll try fixing it before the next release.