Skip to content

Add androidtv_remote.send_text action #139033

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.

Sign up for GitHub

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

Closed
wants to merge 8 commits into from
+213 −8
Closed

Add androidtv_remote.send_text action #139033

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ca6aac7
Add androidtv_remote.send_text service
tronikos Feb 22, 2025
3dd72db
delete
tronikos Feb 22, 2025
5d58778
update string
tronikos Feb 23, 2025
e906bbf
Merge branch 'androidtv_remote_send_text' of https://github.com/troni…
tronikos Feb 23, 2025
55b34aa
Merge branch 'dev' into androidtv_remote_send_text
tronikos Feb 24, 2025
355d836
Merge branch 'dev' into androidtv_remote_send_text
tronikos Apr 8, 2025
4e99220
integration service
tronikos Apr 8, 2025
23900f6
Merge branch 'dev' into androidtv_remote_send_text
tronikos Apr 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions homeassistant/components/androidtv_remote/icons.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"services": {
"send_text": {
"service": "mdi:keyboard"
}
}
}
2 changes: 1 addition & 1 deletion homeassistant/components/androidtv_remote/manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,6 @@
"integration_type": "device",
"iot_class": "local_push",
"loggers": ["androidtvremote2"],
"requirements": ["androidtvremote2==0.1.2"],
"requirements": ["androidtvremote2==0.2.0"],
"zeroconf": ["_androidtvremote2._tcp.local."]
}
24 changes: 24 additions & 0 deletions homeassistant/components/androidtv_remote/remote.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
from collections.abc import Iterable
from typing import Any

from androidtvremote2 import ConnectionClosed
import voluptuous as vol

from homeassistant.components.remote import (
ATTR_ACTIVITY,
ATTR_DELAY_SECS,
Expand All @@ -18,6 +21,8 @@
RemoteEntityFeature,
)
from homeassistant.core import HomeAssistant, callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers import config_validation as cv, entity_platform
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback

from . import AndroidTVRemoteConfigEntry
Expand All @@ -27,6 +32,10 @@
PARALLEL_UPDATES = 0


SERVICE_SEND_TEXT = "send_text"
ATTR_TEXT = "text"


async def async_setup_entry(
hass: HomeAssistant,
config_entry: AndroidTVRemoteConfigEntry,
Expand All @@ -35,6 +44,12 @@ async def async_setup_entry(
"""Set up the Android TV remote entity based on a config entry."""
api = config_entry.runtime_data
async_add_entities([AndroidTVRemoteEntity(api, config_entry)])
platform = entity_platform.async_get_current_platform()
platform.async_register_entity_service(
SERVICE_SEND_TEXT,
{vol.Required(ATTR_TEXT): cv.string},
"send_text",
)
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if this service isn't better suited for just a normal integration service, rather than an entity service. Since in that case you would pick either the config entry or the device, WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it would be more confusing to users since they won't know they can either specify config entry or entity without looking at the documentation and they will end up specifying both. The code will also be more complicated to handle the different selections. Searching for ConfigEntrySelector I only found 12 usages. None of them seem to support config entry or entity/device as target. Anyway it seems we could do what you suggested at any time in the future without being a breaking change so I'd rather stick to the simpler approach for now.

Copy link
Member

Choose a reason for hiding this comment

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

I dont mean to pick both, i just mean to pick a device, rather than an entity. Changing this will be a breaking change as users have to change their automations

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe instead of an action, it would be worth considering introducing a text entity that could also display the current entered text on TV, like the official app does.

Copy link
Member Author

Choose a reason for hiding this comment

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

@joostlek I guess I misunderstood what you are proposing. I personally don't like using a config entry or device in my automations since they are IDs that can change if you remove and re-add the device. I always like using entity IDs. What would the benefit be of your proposal?

@Drafteed the library doesn't support yet getting the current entered text. Somebody needs to reverse engineer that and implement it. I don't have such plans

Copy link

Choose a reason for hiding this comment

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

That sounds good to me, although input text and Lit_ are features of the underlying libraries used rather than Home Assistant design decisions. Mimicking that in the Android TV Remote component wouldn't be a bad idea.

You could also consider using the non-target device field, which seems to be used for IR remotes but not for media platform integrations that use the remote.send_command service.

action: remote.send_command
data:
  command: "{{ text }}"
  device: keyboard
target:
  entity_id: remote.living_room_tv

As someone who hasn't done much work on Home Assistant core, what is the difference between a service that runs on the integration vs entity vs config entry vs device? Is it the target? Service name? I've seen integration/entity/device/config entry service thrown around in this comment thread but don't fully understand the differences between them.

Copy link
Member Author

Choose a reason for hiding this comment

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

I switched to an integration service in this PR. So this now has to be called as:

action: androidtv_remote.send_text
data:
  config_entry_id: 0adab1e29ef310f1d2a97849c911dddd
  text: hi

As an alternative I have #142438. With that you'll call it with:

action: remote.send_command
data:
  device: keyboard
  command: hi
target:
  entity_id: remote.living_room_tv

I prefer the second option.

Copy link

Choose a reason for hiding this comment

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

I also STRONGLY prefer the second option. I can't even think of any services that act on config entry IDs like the first one does. Definitely nothing related to Android TV or any other media platform.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are a few services that act on config entries but all the ones I'm aware of are for integrations that either don't have entities or the services aren't meant to be acted on their entities.

@joostlek you said:

Entity actions should only be used in specific cases where it fits the integration architecture, mostly because they are hard to validate when the device is offline, which leaves a subpar user experience

I think using the existing remote.send_command and device: keyboard better fits the remote platform. As I said earlier you will use this either from a custom card (such as Nerwyn's card) or from an automation that opens an app, sends buttons to navigate to search, sends back to close the keyboard (if your device pops it up), sends text as keyboard input, sends enter, sends buttons to navigate to the results. All these actions are performed on the remote entity and not the media player entity.

Also, as you can see in #142438 we do know when the device is offline as it returns ConnectionClosed.

Copy link
Member Author

Choose a reason for hiding this comment

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

FYI, all the 4 options implemented (at the latest or a previous commit) in the 2 PRs are in https://gist.github.com/tronikos/e5ea2c95c0300cfa1aeb2d4f5c9a2e90

I'd prefer 1 since it makes automations much easier but I'd be fine with either 2 or 3. I really don't want the 4, the one with the unreadable config_entry_id.



class AndroidTVRemoteEntity(AndroidTVRemoteBaseEntity, RemoteEntity):
Expand Down Expand Up @@ -108,3 +123,12 @@ async def async_send_command(self, command: Iterable[str], **kwargs: Any) -> Non
else:
self._send_key_command(single_command, "SHORT")
await asyncio.sleep(delay_secs)

async def send_text(self, text: str) -> None:
"""Send text to Android TV."""
try:
self._api.send_text(text)
except ConnectionClosed as exc:
raise HomeAssistantError(
"Connection to Android TV device is closed"
) from exc
11 changes: 11 additions & 0 deletions homeassistant/components/androidtv_remote/services.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
send_text:
target:
entity:
integration: androidtv_remote
domain: remote
fields:
text:
required: true
example: "hello world"
selector:
text:
12 changes: 12 additions & 0 deletions homeassistant/components/androidtv_remote/strings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,17 @@
}
}
}
},
"services": {
"send_text": {
"name": "Send text",
"description": "Sends text to an Android device.",
"fields": {
"text": {
"name": "Text",
"description": "Text to 'type'"
}
}
}
}
}
2 changes: 1 addition & 1 deletion requirements_all.txt
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion requirements_test_all.txt
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 27 additions & 0 deletions tests/components/androidtv_remote/test_remote.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,23 @@ async def test_remote_send_command_with_hold_secs(
]


async def test_send_text(
hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_api: MagicMock
) -> None:
"""Test sending text via the `androidtv_remote.send_text` service."""
mock_config_entry.add_to_hass(hass)
await hass.config_entries.async_setup(mock_config_entry.entry_id)
assert mock_config_entry.state is ConfigEntryState.LOADED

await hass.services.async_call(
"androidtv_remote",
"send_text",
{"entity_id": REMOTE_ENTITY, "text": "hello world"},
blocking=True,
)
assert mock_api.send_text.mock_calls == [call("hello world")]


async def test_remote_connection_closed(
hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_api: MagicMock
) -> None:
Expand Down Expand Up @@ -205,3 +222,13 @@ async def test_remote_connection_closed(
blocking=True,
)
assert mock_api.send_launch_app_command.mock_calls == [call("activity1")]

mock_api.send_text.side_effect = ConnectionClosed()
with pytest.raises(HomeAssistantError):
await hass.services.async_call(
"androidtv_remote",
"send_text",
{"entity_id": REMOTE_ENTITY, "text": "hello world"},
blocking=True,
)
assert mock_api.send_text.mock_calls == [call("hello world")]