Skip to content
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

Update LG webOS TV IQS #135509

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Conversation

thecode
Copy link
Member

@thecode thecode commented Jan 13, 2025

Proposed change

Docs update PRs:

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • 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:

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • I have followed the perfect PR recommendations
  • The code has been formatted using Ruff (ruff format homeassistant tests)
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • The manifest file has all fields filled out correctly.
    Updated and included derived files by running: python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt.
    Updated by running python3 -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.

To help with the load of incoming pull requests:

Comment on lines +29 to +31
entity-unavailable:
status: exempt
comment: The device does not have an unavailable state (off when not connected)
Copy link
Member

Choose a reason for hiding this comment

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

Isn't it unavailable when someone doesnt have device triggers set up?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, when triggers are not set the feature to power on the device is removed, but you can still power the device externally to the media player or to HA (for example the remote).

if self._turn_on:
return self._supported_features | MediaPlayerEntityFeature.TURN_ON

Copy link
Member

Choose a reason for hiding this comment

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

you can still power the device externally to the media player or to HA (for example the remote).

How does that work?

Copy link
Member Author

Choose a reason for hiding this comment

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

The integration try to connect to the TV every 10 seconds:

with suppress(*WEBOSTV_EXCEPTIONS):
try:
await self._client.connect()

So if the user power on the TV using the original remote (or any other type of power control that is not linked directly to the trigger, such as an IR blaster) and the device state (Idle, Playing, Paused) will be updated.

I had one of my TVs for few months without setting a trigger to power on since it is used by one of my kids and we do not power on the TV remotely, but we do monitor the daily usage or power off the TV remotely.

Copy link
Member

Choose a reason for hiding this comment

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

But during the time it is off (and does not respond to anything) we can set it to unavailable right? Because we also cant do anything with it if the trigger is not used.

I believe samsung tv has this pattern as well

Copy link
Member Author

@thecode thecode Jan 13, 2025

Choose a reason for hiding this comment

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

Why would you want your TV to show as unavailable when it is off? This is a bad user experience, I would rather keep the integration scale without any rating than showing a device that is off as unavailable.

Yes, Samsung TV mark the TV as unavailable when there is no trigger to turn on set, but I think this is an incorrect implementation by Samsung TV and should be fixed. HA already represent that correctly, if you remove the trigger the power/on off button will be removed from the UI so the user understand that he can't turn it on using HA, if we mark it unavailable it will be very confusing to users as they will not understand why the device is unavailable.

If you didn't set a trigger to turn on the device will show that HA doesn't have the capability to turn on the device, but it will still work if you turned it on.

Copy link
Member

Choose a reason for hiding this comment

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

I think my reaction more comes from the fact that almost every other device in HA turns unavailable when we can't connect to it, even if that means the device is off. I get that a TV can be seen as a different kind of device. I think I am more looking for the "what is expected". And maybe I think samsungs implementation is slightly more correct, I am still 50/50 on what is right.

But I do think we should make it consistent in a way (maybe not across devices, but then across TV integrations).

cc @epenet

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry guys - I don't have a hard-set opinion on this.
It just feels like an arch discussion/decision to me.

Samsung has sooo many models with different firmware (and connectivity, etc.) that it's hard to work out the best behavior.
Some recent devices communicate on the network when they are off, therefore indicating that they are not unavailable - just off - so that makes it easier but for the others I don't know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants