-
-
Notifications
You must be signed in to change notification settings - Fork 100
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
New media player service that send a key to the device #299
Comments
That's why we have the remote integration . It is pretty common for integrations to support multiple platforms. |
@balloob If I've understood correctly: instead of adding to media player the send key feature you suggest to implement the https://www.home-assistant.io/integrations/remote (send_command I suppose) at component level. |
I might be the one misunderstanding but:
So the remote integration is used to get events from remotes, and this proposal would provide services available by media players. Ultimately I would create an automation to to trigger such services based on the events of a remote. I think they are opposite sides of the coin. Sorry If I'm not getting this right |
The We don't need to further complicate the media_player integration. |
That's clear @MartinHjelmare. I'm going to implement in a new PR closing the current one.
Regards, |
@MartinHjelmare, @balloob and @dgomes: I do agree with the remote integration suggestion but, IMHO, I think that if we don't define a standard set of commands like I did in my PR for the media_send_key service (where I defined a list of supported keys), it will be useless (in order to have a generic behaviour between platforms) |
What about "simply" moving my media_send_key service from media_player to the remote integration? |
@MartinHjelmare and @balloob what's your opinion? Should I go for a Samsung media player send_key service (that will expose to frontend and the Samsungctl functionality) make a generic send_control_key service at remote level (with fixed generic key set) or use the already defined send_command and implement it for Samsung TVs. |
I suggest making a remote platform for the samsungtv integration. Note that this will require a refactor of the existing media_player platform first. to extract the connection to the samsungtv component. Also note other ongoing samsung PRs. |
@MartinHjelmare. Implementing the remote platform will also require the refractor of the media player platform for sure. IMHO a command is too much generic (it means all and nothing at the same time} ... I would go for a more specific one (maybe with a set of supported basic keys) |
That's my suggestion. |
@MartinHjelmare and @balloob sorry I closed the issue by mistake.
I would go for the third, what is your opinion? |
1 is my suggestion. |
OK. In the send command parameter:
Here's my doubt about the device: I've two samsung TV (media_player.liginvroom and media_player.bedroom) |
Each device should represent its feature set as one or more entities. So if you have two samsung TVs, ie two devices, they should each spawn their own remote entity, representing the remote feature. So two TVs should spawn two remote entities. |
@escoand I created this issue in order to find the best way for the exposal of the Samsungctl send key service. |
AFAIK not possible with samsungctl. Especially the legacy protocol is a one-way. With the websocket I'm not familiar. But there are other libraries which can maybe fill the gap: https://github.com/roberodin/ha-samsungtv-custom |
@tulindo what do you mean by "require a refactor"? |
I think that basically @MartinHjelmare wants to keep common code in a single shared component in order to avoid code duplication. |
@escoand since your PR related to config flow for Samsung TV.has been accepted. Do you think that that part can be put in a shared file in order to reuse it in the remote platform I'd like to implement for samsungtv? |
My merged PR was about automatic protocol/port detection. My next PR (currently in preparation) is about the config flow. |
@escoand What's changed from a user point of view with your PR? (less things to configure by hand I suppose) |
Yes, you just don't need to set the port, but it's still possible. So no breaking changes. |
Clear. Going back in topic: |
@tulindo not sure. I would probably do it with multi inheritance. So we could have one base SamsungTV class and two deriving, maybe To the off topic: also thought about it already. But knowing the mac address does not guarantee power on is working. For my legacy device it's not. Would be interested what @balloob or @MartinHjelmare say to all this. |
I wasn't aware of python supporting multi inheritance. |
I just found that appletv has both media player and remote. And looks like the shared code is in the init.py file |
My code is before initializing the media player. It's the discovery. So it should not get in touch with your suggested changes. Is appletv using global functions or does it use our discussed approach? |
Looks like they use a common class. But I gave just a quick look using my mobile. |
Something to keep in mind is that the use case for this sort of thing goes beyond just sending button presses. Many of these TV's/receivers have websocket/http/etc api's supporting a wide range of commands. The remote.send_command call seems to have some infrared-centric stuff, and "device" as a required parameter doesn't really make sense in this context, since the controlled device will be specified through the entity_id as already discussed. "device" could be abused for what I have called "command_type" in my PR, but in this case at least the remote api would have to be modified to make this parameter optional for send_command. It's also not clear that it makes sense for all of these tv/receiver/etc integrations to have to support the remote api, with the associated redundancy of turn_on and turn_off and associated state with the primary media_player component. So I still think there is an argument for adding a generic command call to the media_player api. |
@bendavid your observations make sense to me.
I think that we need clarifications from @balloob |
The first option doesn't make sense to me. You have 1 remote somebody has maybe 3. Should he config also 3 of the remote entities? The whole remote platform is a bit confusing to me. Why is the remote interesting for HA, it's just a way to communicate with a device. And we already have a way to handle the TV, the media player component.My TV has also buttons at the backside. But nobody would implement a platform for this. An exception could be something like a Logitech Harmony but more in the use of a gateway. |
The harmony has state (activities) and can be controlled (switching activities or sending commands to other devices defined in the harmony configuration but not necessarily in HA), so the remote platform makes perfect sense in that context. |
For context, I work in residential AV installation where customers spend $$ on "logitech harmony on steroids". These systems have a long legacy, are old and buggy, have difficulty with IP commands, etc but what they get right is their "representation of a media system." A media system would be represented as sources (media_player components) that are routed to receivers (av_receiver components or display components) and subsequently routed to a display if the receiver is not also display. A representation of its physical connections. Therefore, when adjusting the volume of the source the media_player component would ask the av_receiver component or display to adjust the volume. This media system has one interface for changing sources which would be handled by the receiver or display. To the end user this representation is monolithic - they have a single interface that controls a single media system in a single location and the controls are consistent and simple regardless of whether their commands are sent to a source, receiver, or display. Most of the community questions around media_player implementations are due to how cumbersome it would be to implement EVERY command available to a particular AV receiver. There are hundreds. @bendavid is suggesting to make available to the end user a means to expose commands that are important to them. In his case a toggling mute, in my case dynamic range control. For other users this is a matter of simply fixing one broken command since they have a slightly different version. The user should be able to expose their command in configuration.yaml: a_media_player_platform:
host: 10.10.10.101
commands:
type: http_get
- name: "Mute"
command: "/goform/formiPhoneAppDirect.xml?RCKSK0410370" Where the end user or community has directed them to the magic IR or telnet bandaid that the API failed to support. |
(As an aside, one of the things that breaks this source, receiver, display model is the advent of "smart tv's" and receivers with built in streaming functionality, such that the display or receiver also acts as a source.) |
@escoand the remote I was meaning in my first option was intended as one virtual device to control more Samsung TVs (even if you're 10 physical remotes at home) |
Coming back to this, there are at least two distinct ways this ~same use case is currently dealt with in home assistant for integrations which are primarily accessed through the media_player domain.
I have personally contributions submitted or to be submitted for denonavr and webostv integrations to address a similar use case. I can see a few possibilities for how to support this going forward a) Add platform dependent service calls in a separate domain for each integration (as for kodi currently) b) Support remote.send_command service in these cases (can we skip implementing redundant state and turn_on/turn_off services in this case?) c) Add send_command or equivalent to media_player domain |
…ton commands, and callback state updates
For a real-world example here is a PR I made which is a good example of service calls that could easily fit in both the remote or media_player domains. (calling play_media to launch youtube app and play a video on an lg smart tv). |
We use the remote integration with platforms for this purpose: https://developers.home-assistant.io/docs/core/entity/remote |
Context
All televisions (but not only TVs) do have a remote controller that basically sends keys to the device in order to control it
Proposal
Add to the media player component a service that sends control keys.
I already made a pull request (home-assistant/core#27587) with the service defined and implement for the samsungtv component
Consequences
The first that came into mind is that a frontend developer will be able to build a remote control to manage every TV.
The text was updated successfully, but these errors were encountered: